Re: [Spice-devel] Spice protocol behind a Firewall

[Uri Lublin]

On 02/19/2017 07:33 PM, Oscar Segarra wrote:

Hi Uri,

I have not been able to find the example you suggest... can you paste
the url of the example?



Hi Oscar,

Disclaimer:
   This is just an example. There may be better more secure ways
   to do it. You should research and decide on a solution
   according to your specific requirements.
   I did not even test the suggested solution.

For example:
http://wiki.squid-cache.org/SquidFaq/SquidAcl under
"Is there an easy way of banning all Destination addresses except one?"

You can configure your squid server to allow only access the
two hosts and specific ports on those hosts and deny the rest.

acl GOOD_HOST dst 10.0.0.1
acl GOOD_HOST dst 10.0.0.2
acl GOOD_PORT port 5900
http_access allow GOOD_HOST
http_access allow GOOT_PORT
http_access deny all

# The last command is not needed according to
# http://www.squid-cache.org/Doc/config/http_access/
# but it does appear in the SquidAcl example

Uri.



2017-02-19 18:23 GMT+01:00 Uri Lublin :
On 02/19/2017 12:50 PM, Oscar Segarra wrote:

Hi Uri,

Is there any public documentation for configuring the http/https
proxy?

In my scenario, I have 2 hypervisors and I don't know exactly how to
redirect each port to each hypervisor.

And regarding your comments, host_ip and host_port (in first and
second
command) belong to the reverse proxy or the hypervisor?

Thanks a lot for your help


One proxy server you can try is squid (squid-cache.org
).
Perhaps one of the examples on its site fits your needs.

In the command below, host is the hypervisor.
If you want to hide the hypervisor ip address and port
perhaps a more sophisticated proxy can be used and that
command line will be a bit different. I never tried it.

Regards,
Uri.


El 19 feb. 2017 10:48 a. m., "Uri Lublin" mailto:u...@redhat.com>>

On 02/19/2017 08:07 AM, Oscar Segarra wrote:

Hi,

First of all, I'd like to say that I'm not sure
enough I'm
writing to
the correct mailing list, I have not been able
to find a common
users
mailing list.

I'm planning to deploy a VDI solution based on
SPICE. I'd like
to grant
access through the Internet to the VDI desktops
but I don't want to
expose the hypervisors to the Internet.

Using virt-viewer or remote-viewer (not the
html5 feature as I
want USB
redirection), is there any trick to make this
scenario work:

/Internet --> FW --> Kind of spice reverse proxy
--> FW -->
Hypervisors
(more than one)./


Hi,

If you have an http/https proxy server, please try:
  SPICE_PROXY=proxy_ip:proxy_port  remote-viewer
host_ip:host_port

Hope that helps,
Uri.





___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Oscar Segarra]

Hi Uri,

Thanks a lot for th example... It looks clarify the security/acl but what
I'd like to know is if is there any known configuration for an scenario
like this:

Hypervisor1 (10.0.0.1)
VM1 (port 5900)
VM2 (port 5901)
Hypervisor2 (10.0.0.2)
VM3 (port 5902)
VM4 (port 5903)

Of course, VMx can be migrated from one hypervisor to the other (even live).

What I'd like is to configure

Internet --> Proxy (listening 5900, 5901, 5902, 5903) --> Hypervisor1 or
Hypervisor2 (where the port is up)

I hope not to be the first one with this requirements :S

Thanks a lot.


2017-02-21 9:42 GMT+01:00 Uri Lublin :

> On 02/19/2017 07:33 PM, Oscar Segarra wrote:
>
>> Hi Uri,
>>
>> I have not been able to find the example you suggest... can you paste
>> the url of the example?
>>
>>
> Hi Oscar,
>
> Disclaimer:
>This is just an example. There may be better more secure ways
>to do it. You should research and decide on a solution
>according to your specific requirements.
>I did not even test the suggested solution.
>
> For example:
> http://wiki.squid-cache.org/SquidFaq/SquidAcl under
> "Is there an easy way of banning all Destination addresses except one?"
>
> You can configure your squid server to allow only access the
> two hosts and specific ports on those hosts and deny the rest.
>
> acl GOOD_HOST dst 10.0.0.1
> acl GOOD_HOST dst 10.0.0.2
> acl GOOD_PORT port 5900
> http_access allow GOOD_HOST
> http_access allow GOOT_PORT
> http_access deny all
>
> # The last command is not needed according to
> # http://www.squid-cache.org/Doc/config/http_access/
> # but it does appear in the SquidAcl example
>
> Uri.
>
>
>> 2017-02-19 18:23 GMT+01:00 Uri Lublin :
>> On 02/19/2017 12:50 PM, Oscar Segarra wrote:
>>
>> Hi Uri,
>>
>> Is there any public documentation for configuring the http/https
>> proxy?
>>
>> In my scenario, I have 2 hypervisors and I don't know exactly how
>> to
>> redirect each port to each hypervisor.
>>
>> And regarding your comments, host_ip and host_port (in first and
>> second
>> command) belong to the reverse proxy or the hypervisor?
>>
>> Thanks a lot for your help
>>
>>
>> One proxy server you can try is squid (squid-cache.org
>> ).
>> Perhaps one of the examples on its site fits your needs.
>>
>> In the command below, host is the hypervisor.
>> If you want to hide the hypervisor ip address and port
>> perhaps a more sophisticated proxy can be used and that
>> command line will be a bit different. I never tried it.
>>
>> Regards,
>> Uri.
>>
>>
>> El 19 feb. 2017 10:48 a. m., "Uri Lublin" > >
>>
>>
>> On 02/19/2017 08:07 AM, Oscar Segarra wrote:
>>
>> Hi,
>>
>> First of all, I'd like to say that I'm not sure
>> enough I'm
>> writing to
>> the correct mailing list, I have not been able
>> to find a common
>> users
>> mailing list.
>>
>> I'm planning to deploy a VDI solution based on
>> SPICE. I'd like
>> to grant
>> access through the Internet to the VDI desktops
>> but I don't want to
>> expose the hypervisors to the Internet.
>>
>> Using virt-viewer or remote-viewer (not the
>> html5 feature as I
>> want USB
>> redirection), is there any trick to make this
>> scenario work:
>>
>> /Internet --> FW --> Kind of spice reverse proxy
>> --> FW -->
>> Hypervisors
>> (more than one)./
>>
>>
>> Hi,
>>
>> If you have an http/https proxy server, please try:
>>   SPICE_PROXY=proxy_ip:proxy_port  remote-viewer
>> host_ip:host_port
>>
>> Hope that helps,
>> Uri.
>>
>>
>>
>>
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Christophe Fergeau]

On Tue, Feb 21, 2017 at 10:04:52AM +0100, Oscar Segarra wrote:
> Hi Uri,
> 
> Thanks a lot for th example... It looks clarify the security/acl but what
> I'd like to know is if is there any known configuration for an scenario
> like this:
> 
> Hypervisor1 (10.0.0.1)
> VM1 (port 5900)
> VM2 (port 5901)
> Hypervisor2 (10.0.0.2)
> VM3 (port 5902)
> VM4 (port 5903)
> 
> Of course, VMx can be migrated from one hypervisor to the other (even live).
> 
> What I'd like is to configure
> 
> Internet --> Proxy (listening 5900, 5901, 5902, 5903) --> Hypervisor1 or
> Hypervisor2 (where the port is up)
> 
> I hope not to be the first one with this requirements :S

I think the squid configuration described in
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Virtualization/3.5/html/Installation_Guide/chap-Proxies.html
covers a similar use case.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Oscar Segarra]

Hi Christophe,

I have already read this document... and I'm not able to translate it to
the scenario I've posted..

In this case I cannot see:

1.- Where configure ports
2.- How squid redirects requests to the correct hypervisor (where VM is
running)
3.- In my environment where I don't have oVirt... what to do with this
command: engine-config -s SpiceProxyDefault=someProxy

Thanks a lot,

2017-02-21 11:24 GMT+01:00 Christophe Fergeau :

> On Tue, Feb 21, 2017 at 10:04:52AM +0100, Oscar Segarra wrote:
> > Hi Uri,
> >
> > Thanks a lot for th example... It looks clarify the security/acl but what
> > I'd like to know is if is there any known configuration for an scenario
> > like this:
> >
> > Hypervisor1 (10.0.0.1)
> > VM1 (port 5900)
> > VM2 (port 5901)
> > Hypervisor2 (10.0.0.2)
> > VM3 (port 5902)
> > VM4 (port 5903)
> >
> > Of course, VMx can be migrated from one hypervisor to the other (even
> live).
> >
> > What I'd like is to configure
> >
> > Internet --> Proxy (listening 5900, 5901, 5902, 5903) --> Hypervisor1 or
> > Hypervisor2 (where the port is up)
> >
> > I hope not to be the first one with this requirements :S
>
> I think the squid configuration described in
> https://access.redhat.com/documentation/en-US/Red_Hat_
> Enterprise_Virtualization/3.5/html/Installation_Guide/chap-Proxies.html
> covers a similar use case.
>
> Christophe
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] Windows drivers for virgl?

[Behrooz Shabani]

Hi All,

I discussed the state of windows drivers on IRC with the developers awhile
back but as not everybody is on the channel, I wanted to ask here as well:

Are you aware of any implementation efforts regarding Windows drivers of
virgl? Maybe someone can provide some insights on how to approach it? We
are willing to help the process.

-- 
regards,
behrooz
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Windows drivers for virgl?

[Yan Vugenfirer]

Hi Benrooz,

I am adding Vadim to the thread. He is working on virtio-gpu for Windows.

Best regards,
Yan.

> On 21 Feb 2017, at 12:39, Behrooz Shabani  wrote:
> 
> Hi All,
> 
> I discussed the state of windows drivers on IRC with the developers awhile 
> back but as not everybody is on the channel, I wanted to ask here as well:
> 
> Are you aware of any implementation efforts regarding Windows drivers of 
> virgl? Maybe someone can provide some insights on how to approach it? We are 
> willing to help the process.
> 
> -- 
> regards,
> behrooz
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Christophe Fergeau]

On Tue, Feb 21, 2017 at 11:30:03AM +0100, Oscar Segarra wrote:
> Hi Christophe,
> 
> I have already read this document... and I'm not able to translate it to
> the scenario I've posted..
> 
> In this case I cannot see:
> 
> 1.- Where configure ports

Not familiar at all with squid, but I'd expect the simple config to
forward connection attempts to the same port that you specified that you
want to connect to.

> 2.- How squid redirects requests to the correct hypervisor (where VM is
> running)

Is this any different than if these were for example http servers
residing on the internal servers? Can't something like
SPICE_PROXY=external-host.example.com:8080 remote-viewer
spice://internal-host:5900
be managed by the squid proxy? The client can connect to the proxy using
the external address, tells it what it wants to connect to, and the
proxy forwards the connection.

> 3.- In my environment where I don't have oVirt... what to do with this
> command: engine-config -s SpiceProxyDefault=someProxy

Just ignore everything after the squid configuration.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Oscar Segarra]

Thanks a lot Christophe for your clarifications...I will try and I will let
you know!

> 2.- How squid redirects requests to the correct hypervisor (where VM is
> running)

Is this any different than if these were for example http servers
residing on the internal servers? Can't something like
SPICE_PROXY=external-host.example.com:8080 remote-viewer
spice://internal-host:5900
be managed by the squid proxy? The client can connect to the proxy using
the external address, tells it what it wants to connect to, and the
proxy forwards the connection.

Regarding to sizing the squid server... has anyone tested how many
Microsoft Windows 7 (or 8, or 10) can be supported by an unique squid proxy
server?

Thanks a lot.


2017-02-21 11:45 GMT+01:00 Christophe Fergeau :

> On Tue, Feb 21, 2017 at 11:30:03AM +0100, Oscar Segarra wrote:
> > Hi Christophe,
> >
> > I have already read this document... and I'm not able to translate it to
> > the scenario I've posted..
> >
> > In this case I cannot see:
> >
> > 1.- Where configure ports
>
> Not familiar at all with squid, but I'd expect the simple config to
> forward connection attempts to the same port that you specified that you
> want to connect to.
>
> > 2.- How squid redirects requests to the correct hypervisor (where VM is
> > running)
>
> Is this any different than if these were for example http servers
> residing on the internal servers? Can't something like
> SPICE_PROXY=external-host.example.com:8080 remote-viewer
> spice://internal-host:5900
> be managed by the squid proxy? The client can connect to the proxy using
> the external address, tells it what it wants to connect to, and the
> proxy forwards the connection.
>
> > 3.- In my environment where I don't have oVirt... what to do with this
> > command: engine-config -s SpiceProxyDefault=someProxy
>
> Just ignore everything after the squid configuration.
>
> Christophe
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Uri Lublin]

On 02/21/2017 11:04 AM, Oscar Segarra wrote:

Hi Uri,

Thanks a lot for th example... It looks clarify the security/acl but
what I'd like to know is if is there any known configuration for an
scenario like this:

Hypervisor1 (10.0.0.1)
VM1 (port 5900)
VM2 (port 5901)
Hypervisor2 (10.0.0.2)
VM3 (port 5902)
VM4 (port 5903)




[1] http://wiki.squid-cache.org/SquidFaq/SquidAcl
After reading "And/Or logic" subsection of [1], a configuration
you can try is (again not even tested):
  acl HOST1 10.0.0.1
  acl HOST2 10.0.0.2
  acl PORT1 5900 5901
  acl PORT2 5902 5903
  http_access allow HOST1 PORT1
  http_access allow HOST2 PORT2
  http_access deny all


Regards,
Uri.



2017-02-21 9:42 GMT+01:00 Uri Lublin mailto:u...@redhat.com>>:

On 02/19/2017 07:33 PM, Oscar Segarra wrote:

Hi Uri,

I have not been able to find the example you suggest... can you
paste
the url of the example?


Hi Oscar,

Disclaimer:
   This is just an example. There may be better more secure ways
   to do it. You should research and decide on a solution
   according to your specific requirements.
   I did not even test the suggested solution.

For example:
http://wiki.squid-cache.org/SquidFaq/SquidAcl
 under
"Is there an easy way of banning all Destination addresses except one?"

You can configure your squid server to allow only access the
two hosts and specific ports on those hosts and deny the rest.

acl GOOD_HOST dst 10.0.0.1
acl GOOD_HOST dst 10.0.0.2
acl GOOD_PORT port 5900
http_access allow GOOD_HOST
http_access allow GOOT_PORT
http_access deny all

# The last command is not needed according to
# http://www.squid-cache.org/Doc/config/http_access/

# but it does appear in the SquidAcl example

Uri.



___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Oscar Segarra]

Hi Uri,

The problem comes when VMs can migrate between Hypervisors. It is,
eventually the scenario can turn as follows:

Hypervisor1 (10.0.0.1) <-- Stopped due to maintenance
Hypervisor2 (10.0.0.2)
VM1 (port 5900)
VM2 (port 5901)
VM3 (port 5902)
VM4 (port 5903)

Thanks a lot!

2017-02-21 13:49 GMT+01:00 Uri Lublin :

> On 02/21/2017 11:04 AM, Oscar Segarra wrote:
>
>> Hi Uri,
>>
>> Thanks a lot for th example... It looks clarify the security/acl but
>> what I'd like to know is if is there any known configuration for an
>> scenario like this:
>>
>> Hypervisor1 (10.0.0.1)
>> VM1 (port 5900)
>> VM2 (port 5901)
>> Hypervisor2 (10.0.0.2)
>> VM3 (port 5902)
>> VM4 (port 5903)
>>
>>
>
> [1] http://wiki.squid-cache.org/SquidFaq/SquidAcl
> After reading "And/Or logic" subsection of [1], a configuration
> you can try is (again not even tested):
>   acl HOST1 10.0.0.1
>   acl HOST2 10.0.0.2
>   acl PORT1 5900 5901
>   acl PORT2 5902 5903
>   http_access allow HOST1 PORT1
>   http_access allow HOST2 PORT2
>   http_access deny all
>
>
> Regards,
> Uri.
>
>
>> 2017-02-21 9:42 GMT+01:00 Uri Lublin > >:
>>
>>
>> On 02/19/2017 07:33 PM, Oscar Segarra wrote:
>>
>> Hi Uri,
>>
>> I have not been able to find the example you suggest... can you
>> paste
>> the url of the example?
>>
>>
>> Hi Oscar,
>>
>> Disclaimer:
>>This is just an example. There may be better more secure ways
>>to do it. You should research and decide on a solution
>>according to your specific requirements.
>>I did not even test the suggested solution.
>>
>> For example:
>> http://wiki.squid-cache.org/SquidFaq/SquidAcl
>>  under
>> "Is there an easy way of banning all Destination addresses except
>> one?"
>>
>> You can configure your squid server to allow only access the
>> two hosts and specific ports on those hosts and deny the rest.
>>
>> acl GOOD_HOST dst 10.0.0.1
>> acl GOOD_HOST dst 10.0.0.2
>> acl GOOD_PORT port 5900
>> http_access allow GOOD_HOST
>> http_access allow GOOT_PORT
>> http_access deny all
>>
>> # The last command is not needed according to
>> # http://www.squid-cache.org/Doc/config/http_access/
>> 
>> # but it does appear in the SquidAcl example
>>
>> Uri.
>>
>>
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Uri Lublin]

On 02/21/2017 02:52 PM, Oscar Segarra wrote:

Hi Uri,

The problem comes when VMs can migrate between Hypervisors. It is,
eventually the scenario can turn as follows:

Hypervisor1 (10.0.0.1) <-- Stopped due to maintenance
Hypervisor2 (10.0.0.2)
VM1 (port 5900)
VM2 (port 5901)
VM3 (port 5902)
VM4 (port 5903)

Thanks a lot!


Hi Oscar,

I do not understand what the problem is.
I think migration would work just fine.

You should configure the setup according to your requirements.
If you want to have 2 VMs running at the same time on
a single host, then the first squid configuration example
may work for you. If you like the number of VMs to be 4
please enable 4 ports (on each host).
If you want different ports enabled on different hosts
than you can try the second example.

Uri.




2017-02-21 13:49 GMT+01:00 Uri Lublin mailto:u...@redhat.com>>:

On 02/21/2017 11:04 AM, Oscar Segarra wrote:

Hi Uri,

Thanks a lot for th example... It looks clarify the security/acl but
what I'd like to know is if is there any known configuration for an
scenario like this:

Hypervisor1 (10.0.0.1)
VM1 (port 5900)
VM2 (port 5901)
Hypervisor2 (10.0.0.2)
VM3 (port 5902)
VM4 (port 5903)



[1] http://wiki.squid-cache.org/SquidFaq/SquidAcl

After reading "And/Or logic" subsection of [1], a configuration
you can try is (again not even tested):
  acl HOST1 10.0.0.1
  acl HOST2 10.0.0.2
  acl PORT1 5900 5901
  acl PORT2 5902 5903
  http_access allow HOST1 PORT1
  http_access allow HOST2 PORT2
  http_access deny all


Regards,
Uri.


2017-02-21 9:42 GMT+01:00 Uri Lublin mailto:u...@redhat.com>
>>:


On 02/19/2017 07:33 PM, Oscar Segarra wrote:

Hi Uri,

I have not been able to find the example you suggest...
can you
paste
the url of the example?


Hi Oscar,

Disclaimer:
   This is just an example. There may be better more secure ways
   to do it. You should research and decide on a solution
   according to your specific requirements.
   I did not even test the suggested solution.

For example:
http://wiki.squid-cache.org/SquidFaq/SquidAcl

> under
"Is there an easy way of banning all Destination addresses
except one?"

You can configure your squid server to allow only access the
two hosts and specific ports on those hosts and deny the rest.

acl GOOD_HOST dst 10.0.0.1
acl GOOD_HOST dst 10.0.0.2
acl GOOD_PORT port 5900
http_access allow GOOD_HOST
http_access allow GOOT_PORT
http_access deny all

# The last command is not needed according to
# http://www.squid-cache.org/Doc/config/http_access/

>
# but it does appear in the SquidAcl example

Uri.





___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Oscar Segarra]

Hi Urii,

What I meant is that VMs can move dynamically bethween hypervisors (or
hosts) and therefore squid configuration may change according to where VMs
are placed on.

What I can do is opening the whole rank 5634 - 6166 (accodring to
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Virtualization/3.1/html/Administration_Guide/Virtualization_Host_Firewall_Requirements1.html)
on each hypervisor.

I will test it in my lab environment and I will let you know.

Have you any advice on the following question:

Regarding to sizing the squid server... has anyone tested how many
Microsoft Windows 7 (or 8, or 10) can be supported by an unique squid proxy
server?

Thanks a lot!


2017-02-21 15:18 GMT+01:00 Uri Lublin :

> On 02/21/2017 02:52 PM, Oscar Segarra wrote:
>
>> Hi Uri,
>>
>> The problem comes when VMs can migrate between Hypervisors. It is,
>> eventually the scenario can turn as follows:
>>
>> Hypervisor1 (10.0.0.1) <-- Stopped due to maintenance
>> Hypervisor2 (10.0.0.2)
>> VM1 (port 5900)
>> VM2 (port 5901)
>> VM3 (port 5902)
>> VM4 (port 5903)
>>
>> Thanks a lot!
>>
>
> Hi Oscar,
>
> I do not understand what the problem is.
> I think migration would work just fine.
>
> You should configure the setup according to your requirements.
> If you want to have 2 VMs running at the same time on
> a single host, then the first squid configuration example
> may work for you. If you like the number of VMs to be 4
> please enable 4 ports (on each host).
> If you want different ports enabled on different hosts
> than you can try the second example.
>
> Uri.
>
>
>
>> 2017-02-21 13:49 GMT+01:00 Uri Lublin > >:
>>
>>
>> On 02/21/2017 11:04 AM, Oscar Segarra wrote:
>>
>> Hi Uri,
>>
>> Thanks a lot for th example... It looks clarify the security/acl
>> but
>> what I'd like to know is if is there any known configuration for
>> an
>> scenario like this:
>>
>> Hypervisor1 (10.0.0.1)
>> VM1 (port 5900)
>> VM2 (port 5901)
>> Hypervisor2 (10.0.0.2)
>> VM3 (port 5902)
>> VM4 (port 5903)
>>
>>
>>
>> [1] http://wiki.squid-cache.org/SquidFaq/SquidAcl
>> 
>> After reading "And/Or logic" subsection of [1], a configuration
>> you can try is (again not even tested):
>>   acl HOST1 10.0.0.1
>>   acl HOST2 10.0.0.2
>>   acl PORT1 5900 5901
>>   acl PORT2 5902 5903
>>   http_access allow HOST1 PORT1
>>   http_access allow HOST2 PORT2
>>   http_access deny all
>>
>>
>> Regards,
>> Uri.
>>
>>
>> 2017-02-21 9:42 GMT+01:00 Uri Lublin > 
>> >>:
>>
>>
>>
>> On 02/19/2017 07:33 PM, Oscar Segarra wrote:
>>
>> Hi Uri,
>>
>> I have not been able to find the example you suggest...
>> can you
>> paste
>> the url of the example?
>>
>>
>> Hi Oscar,
>>
>> Disclaimer:
>>This is just an example. There may be better more secure
>> ways
>>to do it. You should research and decide on a solution
>>according to your specific requirements.
>>I did not even test the suggested solution.
>>
>> For example:
>> http://wiki.squid-cache.org/SquidFaq/SquidAcl
>> 
>> > > under
>> "Is there an easy way of banning all Destination addresses
>> except one?"
>>
>> You can configure your squid server to allow only access the
>> two hosts and specific ports on those hosts and deny the rest.
>>
>> acl GOOD_HOST dst 10.0.0.1
>> acl GOOD_HOST dst 10.0.0.2
>> acl GOOD_PORT port 5900
>> http_access allow GOOD_HOST
>> http_access allow GOOT_PORT
>> http_access deny all
>>
>> # The last command is not needed according to
>> # http://www.squid-cache.org/Doc/config/http_access/
>> 
>> > >
>> # but it does appear in the SquidAcl example
>>
>> Uri.
>>
>>
>>
>>
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v2 0/4] spice-channel read/flush wire functions

[Victor Toso]

Hi,

On Fri, Feb 03, 2017 at 04:13:36PM +0100, Victor Toso wrote:
> From: Victor Toso 
>
> Changes v1->v2:
> * using g_assert instead of g_assert_nonnull
> * Adding https://bugs.freedesktop.org/show_bug.cgi?id=96598 in commit
> * changed shortlog from patches 02/04 and 04/04

Friendly ping (not really urgent)

>
> Victor Toso (4):
>   spice-channel: move out non blocking logic of _read_wire()
>   spice-channel: spice_channel_read_wire() improvements
>   spice-channel: move out non blocking logic of _flush_wire()
>   spice-channel: make _flush_wire() compatible to _read_wire()
> 
>  src/spice-channel.c | 174 
> +---
>  1 file changed, 112 insertions(+), 62 deletions(-)
> 
> -- 
> 2.9.3
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately

[Snir Sheriber]

Hi,


On 02/20/2017 07:00 PM, Christophe de Dinechin wrote:

On 19 Feb 2017, at 15:47, Snir Sheriber  wrote:

Remove handling with failures in the SASL authentication
process to separate function
---
src/spice-channel.c | 44 +++-
1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/spice-channel.c b/src/spice-channel.c
index af67931..cbf1291 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1113,28 +1113,38 @@ static int spice_channel_read(SpiceChannel *channel, 
void *data, size_t length)
 return length;
}

+#if HAVE_SASL
/* coroutine context */
-static void spice_channel_failed_authentication(SpiceChannel *channel,
-gboolean invalidPassword)
+static void spice_channel_failed_sasl_authentication(SpiceChannel *channel)
{
 SpiceChannelPrivate *c = channel->priv;
+gint err_code; /* Affects the authentication window activated fileds */

 if (c->auth_needs_username && c->auth_needs_password)
-g_set_error_literal(&c->error,
-SPICE_CLIENT_ERROR,
-
SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
-_("Authentication failed: password and username are 
required"));
+err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME;
 else if (c->auth_needs_username)
-g_set_error_literal(&c->error,
-SPICE_CLIENT_ERROR,
-SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
-_("Authentication failed: username is required"));
-else if (c->auth_needs_password)
-g_set_error_literal(&c->error,
-SPICE_CLIENT_ERROR,
-SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
-_("Authentication failed: password is required"));
-else if (invalidPassword)
+err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME;
+else
+err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD;
+
+g_set_error_literal(&c->error,
+SPICE_CLIENT_ERROR,
+err_code,
+_("SASL authentication failed"));

Per the recent discussion (Feb 14 with Christophe F), can’t we map common SASL 
errors to Spice messages? To me, it’s different if the problem is that I used a 
wrong password or if the server is down. The message as is seems quite terse.

Errors that seem be reportable (although not all of them seem relevant to 
Spice):

SASL_BADAUTHAuthentication failure.
SASL_NOAUTHZAuthorization failure.
SASL_EXPIREDThe passphrase expired and must be reset.
SASL_DISABLED   Account disabled.
SASL_NOUSER User not found.
SASL_BADVERSVersion mismatch with plug-in.
SASL_NOVERIFY   The user exists, but there is no verifier for the user.
SASL_WEAKPASS   The passphrase is too weak for security policy.
SASL_NOUSERPASS User supplied passwords are not permitted.


Some that may need to be “translated” in Spicese if they ever get back to us:

SASL_TOOWEAKThe mechanism is too weak for this user.
SASL_ENCRYPTEncryption is needed to use this mechanism.
SASL_TRANS  One time use of a plaintext password will enable 
requested mechanism for user.

Others should probably collected into a “default” in a switch statement, something 
like “Unexpected SASL error code ”.

As link result/error? I guess it would be the best , but first it 
requires to inform the client in some other way that it can stop waiting 
for the sasl server start\step result (currently it just freeing the link)

btw according to sasl docs the full error string should be sent

+
+c->event = SPICE_CHANNEL_ERROR_AUTH;
+
+c->has_error = TRUE; /* force disconnect */
+}
+#endif
+
+/* coroutine context */
+static void spice_channel_failed_authentication(SpiceChannel *channel,
+gboolean invalidPassword)
+{
+SpiceChannelPrivate *c = channel->priv;
+
+if (invalidPassword)
 g_set_error_literal(&c->error,
 SPICE_CLIENT_ERROR,
 SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
@@ -1808,7 +1818,7 @@ error:
 if (saslconn)
 sasl_dispose(&saslconn);

-spice_channel_failed_authentication(channel, FALSE);
+spice_channel_failed_sasl_authentication(channel);
 ret = FALSE;

cleanup:
--
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Oscar Segarra]

Hi Uri,

If you change the squid configuration dynamically you'll need to
make sure a client can to the VM. That may include migration.


What would be interesting is habing a kind of health check that checks
"Port telnet". And if a port is ready, add dinamically the rule to the
squid server.

Sorry, I do not have an answer for that.
Of course it depends on the hardware of the servers, the network etc.

If you test it, please let us know what numbers you get.


My lab enviroment I think is not good enough for this kind of tests
(windows10 --> vmware workstations --> kvm --> windows 10). But I will let
you know when I have it in production environment.

Thanks a lot.


2017-02-21 16:16 GMT+01:00 Uri Lublin :

> On 02/21/2017 04:44 PM, Oscar Segarra wrote:
>
>> Hi Urii,
>>
>> What I meant is that VMs can move dynamically bethween hypervisors (or
>> hosts) and therefore squid configuration may change according to where
>> VMs are placed on.
>>
>
>
> If you change the squid configuration dynamically you'll need to
> make sure a client can to the VM. That may include migration.
>
> What I can do is opening the whole rank 5634 - 6166 (accodring to
>> https://access.redhat.com/documentation/en-US/Red_Hat_Enterp
>> rise_Virtualization/3.1/html/Administration_Guide/Virtualiz
>> ation_Host_Firewall_Requirements1.html)
>> on each hypervisor.
>>
>
> You can do that.
> The number of ports "opened" depends on your requirements.
>
> I will test it in my lab environment and I will let you know.
>>
>> Have you any advice on the following question:
>>
>> Regarding to sizing the squid server... has anyone tested how many
>> Microsoft Windows 7 (or 8, or 10) can be supported by an unique squid
>> proxy server?
>>
>
> Sorry, I do not have an answer for that.
> Of course it depends on the hardware of the servers, the network etc.
>
> If you test it, please let us know what numbers you get.
>
> Thanks,
> Uri.
>
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Spice protocol behind a Firewall

[Uri Lublin]

On 02/21/2017 04:44 PM, Oscar Segarra wrote:

Hi Urii,

What I meant is that VMs can move dynamically bethween hypervisors (or
hosts) and therefore squid configuration may change according to where
VMs are placed on.



If you change the squid configuration dynamically you'll need to
make sure a client can to the VM. That may include migration.


What I can do is opening the whole rank 5634 - 6166 (accodring to
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Virtualization/3.1/html/Administration_Guide/Virtualization_Host_Firewall_Requirements1.html)
on each hypervisor.


You can do that.
The number of ports "opened" depends on your requirements.


I will test it in my lab environment and I will let you know.

Have you any advice on the following question:

Regarding to sizing the squid server... has anyone tested how many
Microsoft Windows 7 (or 8, or 10) can be supported by an unique squid
proxy server?


Sorry, I do not have an answer for that.
Of course it depends on the hardware of the servers, the network etc.

If you test it, please let us know what numbers you get.

Thanks,
Uri.

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v1] gtk-session: Use GWeakRef

[Christophe Fergeau]

On Mon, Feb 20, 2017 at 03:49:02PM +0100, Victor Toso wrote:
> From: Victor Toso 
> 
> The custom WeakRef structure was introduced in 9baba9fd89 (2012) to
> fix rhbz#743773. Since glib 2.32, GWeakRef was introduced and it
> behaves similarly to our WeakRef.
> 
> Moving to GWeakRef to remove some code which exists in glib.
> 
> Note that I'm keeping to utility functions:
> - get_weak_ref(gpointer object): which returns a newly allocated
>   GWeakRef, initialized with @object;
> - free_weak_ref(gpointer pdata): which frees the GWeakRef and returns
>   the original object or NULL. It also takes care of an extra
>   reference to the object.

Yes, this dropping of the strong ref is a bit odd at first, but in the
end it's no different than how the code was working before, we expect
the object to stay alive for the duration of the callback if it was
alive when entering in the callback.

> 
> Signed-off-by: Victor Toso 
> ---
>  src/spice-gtk-session.c | 52 
> +++--
>  1 file changed, 20 insertions(+), 32 deletions(-)
> 
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 5688cba..88f4488 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -575,31 +575,26 @@ static const struct {
>  }
>  };
>  
> -typedef struct _WeakRef {
> -GObject *object;
> -} WeakRef;
> -
> -static void weak_notify_cb(WeakRef *weakref, GObject *object)
> -{
> -weakref->object = NULL;
> -}
> -
> -static WeakRef* weak_ref(GObject *object)
> +static GWeakRef* get_weak_ref(gpointer object)
>  {
> -WeakRef *weakref = g_new(WeakRef, 1);
> -
> -g_object_weak_ref(object, (GWeakNotify)weak_notify_cb, weakref);
> -weakref->object = object;
> -
> +GWeakRef *weakref = g_new(GWeakRef, 1);
> +g_weak_ref_init(weakref, object);
>  return weakref;
>  }
>  
> -static void weak_unref(WeakRef* weakref)
> +static gpointer free_weak_ref(gpointer data)
>  {
> -if (weakref->object)
> -g_object_weak_unref(weakref->object, (GWeakNotify)weak_notify_cb, 
> weakref);
> +GWeakRef *weakref = data;
> +gpointer object = g_weak_ref_get(weakref);
>  
> +g_weak_ref_clear(weakref);
>  g_free(weakref);
> +if (object != NULL) {
> +/* The main referece still exists as object is not NULL, so we can

'reference'

Looks good to me, though I'd probably return a ref to the callers, and
add a g_object_unref() to the end of the callbacks.

Acked-by: Christophe Fergeau 


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately

[Christophe Fergeau]

On Sun, Feb 19, 2017 at 04:47:17PM +0200, Snir Sheriber wrote:
> Remove handling with failures in the SASL authentication
> process to separate function
> ---
>  src/spice-channel.c | 44 +++-
>  1 file changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index af67931..cbf1291 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -1113,28 +1113,38 @@ static int spice_channel_read(SpiceChannel *channel, 
> void *data, size_t length)
>  return length;
>  }
>  
> +#if HAVE_SASL
>  /* coroutine context */
> -static void spice_channel_failed_authentication(SpiceChannel *channel,
> -gboolean invalidPassword)
> +static void spice_channel_failed_sasl_authentication(SpiceChannel *channel)
>  {
>  SpiceChannelPrivate *c = channel->priv;
> +gint err_code; /* Affects the authentication window activated fileds */
>  
>  if (c->auth_needs_username && c->auth_needs_password)
> -g_set_error_literal(&c->error,
> -SPICE_CLIENT_ERROR,
> -
> SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
> -_("Authentication failed: password and username 
> are required"));
> +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME;
>  else if (c->auth_needs_username)
> -g_set_error_literal(&c->error,
> -SPICE_CLIENT_ERROR,
> -SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
> -_("Authentication failed: username is 
> required"));
> -else if (c->auth_needs_password)
> -g_set_error_literal(&c->error,
> -SPICE_CLIENT_ERROR,
> -SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
> -_("Authentication failed: password is 
> required"));
> -else if (invalidPassword)
> +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME;
> +else
> +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD;
> +
> +g_set_error_literal(&c->error,
> +SPICE_CLIENT_ERROR,
> +err_code,
> +_("SASL authentication failed"));

I don't understand what you are aiming for with this series. I thought
the goal was to get better error messages on autentication failures, but
this change merges 3 distinct error messages
_("Authentication failed: password and username are required"));
_("Authentication failed: username is required"));
_("Authentication failed: password is required"));

to just

_("SASL authentication failed"));

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v2 4/4] spice-channel: make _flush_wire() compatible to _read_wire()

[Christophe Fergeau]

On Fri, Feb 03, 2017 at 04:13:40PM +0100, Victor Toso wrote:
> From: Victor Toso 
> 
> * Keeping compatibility with spice_channel_read_wire() where we keep
I would not talk about 'compatibility", just saying they are similar ?

>   the possible context switch of g_coroutine_socket_wait() in the end
>   of loop and dealing with the return of the _flush_wire_nonblocking()
>   case by case.
> 
> Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598
> Signed-off-by: Victor Toso 
> ---
>  src/spice-channel.c | 26 ++
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index 9e43c6c..55d3494 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -832,26 +832,28 @@ static void spice_channel_flush_wire(SpiceChannel 
> *channel,
>  while (offset < datalen) {
>  gssize ret;
>  
> -if (c->has_error) return;
> +if (c->has_error)
> +return;
>  
>  ret = spice_channel_flush_wire_nonblocking(channel, ptr+offset, 
> datalen-offset, &cond);
> -if (ret == -1) {
> -if (cond != 0) {
> -// TODO: should use 
> g_pollable_input/output_stream_create_source() in 2.28 ?
> -g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> -continue;
> -} else {
> -CHANNEL_DEBUG(channel, "Closing the channel: 
> spice_channel_flush %d", errno);
> -c->has_error = TRUE;
> -return;
> -}
> +if (ret > 0) {
> +offset += ret;
> +continue;
>  }
> +
>  if (ret == 0) {
>  CHANNEL_DEBUG(channel, "Closing the connection: 
> spice_channel_flush");
>  c->has_error = TRUE;
>  return;
>  }
> -offset += ret;
> +
> +if (cond == 0) {

Fwiw, I think I find the initial version a bit easier to follow as all
the toplevel testing is checking 'ret' value, and then you look at other
things (cond).
One thing I'd add though is a bunch of else so that it's clear that the
various cases are exclusive.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v2 2/4] spice-channel: spice_channel_read_wire() improvements

[Christophe Fergeau]

Looks good

Acked-by: Christophe Fergeau 

On Fri, Feb 03, 2017 at 04:13:38PM +0100, Victor Toso wrote:
> From: Victor Toso 
> 
> * Removes the reread label by having while(TRUE);
> 
>   The label is being used after g_coroutine_socket_wait() is called,
>   which is context switch while we can't do the reading as it would
>   block. Moving this inside a while() makes the code more straight
>   forward to be read and should not impact the performance.
> 
> * Move local variables inside the block they will be used.
> 
> Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598
> Signed-off-by: Victor Toso 
> ---
>  src/spice-channel.c | 38 --
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index a17c402..d1714df 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -1025,32 +1025,34 @@ static int 
> spice_channel_read_wire_nonblocking(SpiceChannel *channel,
>  static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t 
> len)
>  {
>  SpiceChannelPrivate *c = channel->priv;
> -GIOCondition cond;
> -gssize ret;
>  
> -reread:
> +while (TRUE) {
> +gssize ret;
> +GIOCondition cond;
>  
> -if (c->has_error) return 0; /* has_error is set by disconnect(), return 
> no error */
> +if (c->has_error) {
> +/* has_error is set by disconnect(), return no error */
> +return 0;
> +}
>  
> -ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
> +ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
> +if (ret > 0)
> +return ret;
>  
> -if (ret == -1) {
> -if (cond != 0) {
> -// TODO: should use 
> g_pollable_input/output_stream_create_source() ?
> -g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> -goto reread;
> -} else {
> +if (ret == 0) {
> +CHANNEL_DEBUG(channel, "Closing the connection: 
> spice_channel_read() - ret=0");
> +c->has_error = TRUE;
> +return 0;
> +}
> +
> +if (cond == 0) {
>  c->has_error = TRUE;
>  return -errno;
>  }
> -}
> -if (ret == 0) {
> -CHANNEL_DEBUG(channel, "Closing the connection: spice_channel_read() 
> - ret=0");
> -c->has_error = TRUE;
> -return 0;
> -}
>  
> -return ret;
> +// TODO: should use g_pollable_input/output_stream_create_source() ?
> +g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> +}
>  }
>  
>  #ifdef HAVE_SASL
> -- 
> 2.9.3
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v2 2/4] spice-channel: spice_channel_read_wire() improvements

[Christophe Fergeau]

On Tue, Feb 21, 2017 at 06:11:54PM +0100, Christophe Fergeau wrote:
> Looks good
> 
> Acked-by: Christophe Fergeau 

I take that ACK back, this was meant to be sent for 3/4 !

Christophe

> 
> On Fri, Feb 03, 2017 at 04:13:38PM +0100, Victor Toso wrote:
> > From: Victor Toso 
> > 
> > * Removes the reread label by having while(TRUE);
> > 
> >   The label is being used after g_coroutine_socket_wait() is called,
> >   which is context switch while we can't do the reading as it would
> >   block. Moving this inside a while() makes the code more straight
> >   forward to be read and should not impact the performance.
> > 
> > * Move local variables inside the block they will be used.
> > 
> > Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598
> > Signed-off-by: Victor Toso 
> > ---
> >  src/spice-channel.c | 38 --
> >  1 file changed, 20 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/spice-channel.c b/src/spice-channel.c
> > index a17c402..d1714df 100644
> > --- a/src/spice-channel.c
> > +++ b/src/spice-channel.c
> > @@ -1025,32 +1025,34 @@ static int 
> > spice_channel_read_wire_nonblocking(SpiceChannel *channel,
> >  static int spice_channel_read_wire(SpiceChannel *channel, void *data, 
> > size_t len)
> >  {
> >  SpiceChannelPrivate *c = channel->priv;
> > -GIOCondition cond;
> > -gssize ret;
> >  
> > -reread:
> > +while (TRUE) {
> > +gssize ret;
> > +GIOCondition cond;
> >  
> > -if (c->has_error) return 0; /* has_error is set by disconnect(), 
> > return no error */
> > +if (c->has_error) {
> > +/* has_error is set by disconnect(), return no error */
> > +return 0;
> > +}
> >  
> > -ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
> > +ret = spice_channel_read_wire_nonblocking(channel, data, len, 
> > &cond);
> > +if (ret > 0)
> > +return ret;
> >  
> > -if (ret == -1) {
> > -if (cond != 0) {
> > -// TODO: should use 
> > g_pollable_input/output_stream_create_source() ?
> > -g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> > -goto reread;
> > -} else {
> > +if (ret == 0) {
> > +CHANNEL_DEBUG(channel, "Closing the connection: 
> > spice_channel_read() - ret=0");
> > +c->has_error = TRUE;
> > +return 0;
> > +}
> > +
> > +if (cond == 0) {
> >  c->has_error = TRUE;
> >  return -errno;
> >  }
> > -}
> > -if (ret == 0) {
> > -CHANNEL_DEBUG(channel, "Closing the connection: 
> > spice_channel_read() - ret=0");
> > -c->has_error = TRUE;
> > -return 0;
> > -}
> >  
> > -return ret;
> > +// TODO: should use g_pollable_input/output_stream_create_source() 
> > ?
> > +g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> > +}
> >  }
> >  
> >  #ifdef HAVE_SASL
> > -- 
> > 2.9.3
> > 
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel




signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v2 3/4] spice-channel: move out non blocking logic of _flush_wire()

[Christophe Fergeau]

Ok, 
Acked-by: Christophe Fergeau 

On Fri, Feb 03, 2017 at 04:13:39PM +0100, Victor Toso wrote:
> From: Victor Toso 
> 
> This patch introduces spice_channel_flush_wire_nonblocking() helper
> without changing any logic.
> 
> Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598
> Signed-off-by: Victor Toso 
> ---
>  src/spice-channel.c | 73 
> +++--
>  1 file changed, 48 insertions(+), 25 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index d1714df..9e43c6c 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -769,6 +769,53 @@ void spice_msg_out_send_internal(SpiceMsgOut *out)
>  }
>  
>  /*
> + * Helper function to deal with the nonblocking part of _flush_wire() 
> function.
> + * It returns the result of the write and will set the proper bits in @cond 
> in
> + * case the write function would block.
> + *
> + * Returns -1 in case of any problems.
> + */
> +/* coroutine context */
> +static gint spice_channel_flush_wire_nonblocking(SpiceChannel *channel,
> + const gchar *ptr,
> + size_t len,
> + GIOCondition *cond)
> +{
> +SpiceChannelPrivate *c = channel->priv;
> +gssize ret;
> +
> +g_assert(cond != NULL);
> +*cond = 0;
> +
> +if (c->tls) {
> +ret = SSL_write(c->ssl, ptr, len);
> +if (ret < 0) {
> +ret = SSL_get_error(c->ssl, ret);
> +if (ret == SSL_ERROR_WANT_READ)
> +*cond |= G_IO_IN;
> +if (ret == SSL_ERROR_WANT_WRITE)
> +*cond |= G_IO_OUT;
> +ret = -1;
> +}
> +} else {
> +GError *error = NULL;
> +ret = 
> g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(c->out),
> + ptr, len, NULL, 
> &error);
> +if (ret < 0) {
> +if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
> +*cond = G_IO_OUT;
> +} else {
> +CHANNEL_DEBUG(channel, "Send error %s", error->message);
> +}
> +g_clear_error(&error);
> +ret = -1;
> +}
> +}
> +
> +return ret;
> +}
> +
> +/*
>   * Write all 'data' of length 'datalen' bytes out to
>   * the wire
>   */
> @@ -784,34 +831,10 @@ static void spice_channel_flush_wire(SpiceChannel 
> *channel,
>  
>  while (offset < datalen) {
>  gssize ret;
> -GError *error = NULL;
>  
>  if (c->has_error) return;
>  
> -cond = 0;
> -if (c->tls) {
> -ret = SSL_write(c->ssl, ptr+offset, datalen-offset);
> -if (ret < 0) {
> -ret = SSL_get_error(c->ssl, ret);
> -if (ret == SSL_ERROR_WANT_READ)
> -cond |= G_IO_IN;
> -if (ret == SSL_ERROR_WANT_WRITE)
> -cond |= G_IO_OUT;
> -ret = -1;
> -}
> -} else {
> -ret = 
> g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(c->out),
> - ptr+offset, 
> datalen-offset, NULL, &error);
> -if (ret < 0) {
> -if (g_error_matches(error, G_IO_ERROR, 
> G_IO_ERROR_WOULD_BLOCK)) {
> -cond = G_IO_OUT;
> -} else {
> -CHANNEL_DEBUG(channel, "Send error %s", error->message);
> -}
> -g_clear_error(&error);
> -ret = -1;
> -}
> -}
> +ret = spice_channel_flush_wire_nonblocking(channel, ptr+offset, 
> datalen-offset, &cond);
>  if (ret == -1) {
>  if (cond != 0) {
>  // TODO: should use 
> g_pollable_input/output_stream_create_source() in 2.28 ?
> -- 
> 2.9.3
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v2 2/4] spice-channel: spice_channel_read_wire() improvements

[Christophe Fergeau]

On Fri, Feb 03, 2017 at 04:13:38PM +0100, Victor Toso wrote:
> From: Victor Toso 
> 
> * Removes the reread label by having while(TRUE);
> 
>   The label is being used after g_coroutine_socket_wait() is called,
>   which is context switch while we can't do the reading as it would
>   block. Moving this inside a while() makes the code more straight
>   forward to be read and should not impact the performance.
> 
> * Move local variables inside the block they will be used.
> 
> Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598
> Signed-off-by: Victor Toso 
> ---
>  src/spice-channel.c | 38 --
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index a17c402..d1714df 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -1025,32 +1025,34 @@ static int 
> spice_channel_read_wire_nonblocking(SpiceChannel *channel,
>  static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t 
> len)
>  {
>  SpiceChannelPrivate *c = channel->priv;
> -GIOCondition cond;
> -gssize ret;
>  
> -reread:
> +while (TRUE) {
> +gssize ret;
> +GIOCondition cond;
>  
> -if (c->has_error) return 0; /* has_error is set by disconnect(), return 
> no error */
> +if (c->has_error) {
> +/* has_error is set by disconnect(), return no error */
> +return 0;
> +}
>  
> -ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
> +ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
> +if (ret > 0)
> +return ret;
>  
> -if (ret == -1) {
> -if (cond != 0) {
> -// TODO: should use 
> g_pollable_input/output_stream_create_source() ?
> -g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> -goto reread;
> -} else {
> +if (ret == 0) {
> +CHANNEL_DEBUG(channel, "Closing the connection: 
> spice_channel_read() - ret=0");
> +c->has_error = TRUE;
> +return 0;
> +}
> +
> +if (cond == 0) {
>  c->has_error = TRUE;
>  return -errno;
>  }
> -}
> -if (ret == 0) {
> -CHANNEL_DEBUG(channel, "Closing the connection: spice_channel_read() 
> - ret=0");
> -c->has_error = TRUE;
> -return 0;
> -}
>  
> -return ret;
> +// TODO: should use g_pollable_input/output_stream_create_source() ?
> +g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> +}

So basically what this change is doing is moving all the checks leading
to early returns first, and then it handles the remaining case, which
consists in waiting for data availability. As with patch 4/4, I regret
that we change for toplevel tests checking only for 'ret' value to
toplevel tests testing either 'ret' or 'cond'. I'd also add a bunch of
'else' while you are at it so that it's explicit that everything is
mutually exclusive.
If I were writing these patches, I might even go as far as adding a
first commit doing a switch to while(TRUE) and replacing the 'goto' with
'continue', and doing the reformatting, and then have another commit
which moves around the various conditions.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v2 1/4] spice-channel: move out non blocking logic of _read_wire()

[Christophe Fergeau]

If you need this, this looks good,

Acked-by: Christophe Fergeau 

On Fri, Feb 03, 2017 at 04:13:37PM +0100, Victor Toso wrote:
> From: Victor Toso 
> 
> This patch introduces spice_channel_read_wire_nonblocking() helper
> without changing any logic.
> 
> Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598
> Signed-off-by: Victor Toso 
> ---
>  src/spice-channel.c | 45 ++---
>  1 file changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index 6556db3..a17c402 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -971,29 +971,32 @@ gint spice_channel_unix_read_fd(SpiceChannel *channel)
>  #endif
>  
>  /*
> - * Read at least 1 more byte of data straight off the wire
> - * into the requested buffer.
> + * Helper function to deal with the nonblocking part of _read_wire() 
> function.
> + * It returns the result of the read and will set the proper bits in @cond in
> + * case the read function would block.
> + *
> + * Returns -1 in case of any problems.
>   */
>  /* coroutine context */
> -static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t 
> len)
> +static int spice_channel_read_wire_nonblocking(SpiceChannel *channel,
> +   void *data,
> +   size_t len,
> +   GIOCondition *cond)
>  {
>  SpiceChannelPrivate *c = channel->priv;
>  gssize ret;
> -GIOCondition cond;
> -
> -reread:
>  
> -if (c->has_error) return 0; /* has_error is set by disconnect(), return 
> no error */
> +g_assert(cond != NULL);
> +*cond = 0;
>  
> -cond = 0;
>  if (c->tls) {
>  ret = SSL_read(c->ssl, data, len);
>  if (ret < 0) {
>  ret = SSL_get_error(c->ssl, ret);
>  if (ret == SSL_ERROR_WANT_READ)
> -cond |= G_IO_IN;
> +*cond |= G_IO_IN;
>  if (ret == SSL_ERROR_WANT_WRITE)
> -cond |= G_IO_OUT;
> +*cond |= G_IO_OUT;
>  ret = -1;
>  }
>  } else {
> @@ -1002,7 +1005,7 @@ reread:
> data, len, NULL, 
> &error);
>  if (ret < 0) {
>  if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
> -cond = G_IO_IN;
> +*cond = G_IO_IN;
>  } else {
>  CHANNEL_DEBUG(channel, "Read error %s", error->message);
>  }
> @@ -1011,6 +1014,26 @@ reread:
>  }
>  }
>  
> +return ret;
> +}
> +
> +/*
> + * Read at least 1 more byte of data straight off the wire
> + * into the requested buffer.
> + */
> +/* coroutine context */
> +static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t 
> len)
> +{
> +SpiceChannelPrivate *c = channel->priv;
> +GIOCondition cond;
> +gssize ret;
> +
> +reread:
> +
> +if (c->has_error) return 0; /* has_error is set by disconnect(), return 
> no error */
> +
> +ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
> +
>  if (ret == -1) {
>  if (cond != 0) {
>  // TODO: should use 
> g_pollable_input/output_stream_create_source() ?
> -- 
> 2.9.3
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v1] gtk-session: Use GWeakRef

[Victor Toso]

Hi,

On Tue, Feb 21, 2017 at 05:16:26PM +0100, Christophe Fergeau wrote:
> On Mon, Feb 20, 2017 at 03:49:02PM +0100, Victor Toso wrote:
> > From: Victor Toso 
> > 
> > The custom WeakRef structure was introduced in 9baba9fd89 (2012) to
> > fix rhbz#743773. Since glib 2.32, GWeakRef was introduced and it
> > behaves similarly to our WeakRef.
> > 
> > Moving to GWeakRef to remove some code which exists in glib.
> > 
> > Note that I'm keeping to utility functions:

Fixed typo: to -> two

> > - get_weak_ref(gpointer object): which returns a newly allocated
> >   GWeakRef, initialized with @object;
> > - free_weak_ref(gpointer pdata): which frees the GWeakRef and returns
> >   the original object or NULL. It also takes care of an extra
> >   reference to the object.
>
> Yes, this dropping of the strong ref is a bit odd at first, but in the
> end it's no different than how the code was working before, we expect
> the object to stay alive for the duration of the callback if it was
> alive when entering in the callback.
> 
> > 
> > Signed-off-by: Victor Toso 
> > ---
> >  src/spice-gtk-session.c | 52 
> > +++--
> >  1 file changed, 20 insertions(+), 32 deletions(-)
> > 
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index 5688cba..88f4488 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -575,31 +575,26 @@ static const struct {
> >  }
> >  };
> >  
> > -typedef struct _WeakRef {
> > -GObject *object;
> > -} WeakRef;
> > -
> > -static void weak_notify_cb(WeakRef *weakref, GObject *object)
> > -{
> > -weakref->object = NULL;
> > -}
> > -
> > -static WeakRef* weak_ref(GObject *object)
> > +static GWeakRef* get_weak_ref(gpointer object)
> >  {
> > -WeakRef *weakref = g_new(WeakRef, 1);
> > -
> > -g_object_weak_ref(object, (GWeakNotify)weak_notify_cb, weakref);
> > -weakref->object = object;
> > -
> > +GWeakRef *weakref = g_new(GWeakRef, 1);
> > +g_weak_ref_init(weakref, object);
> >  return weakref;
> >  }
> >  
> > -static void weak_unref(WeakRef* weakref)
> > +static gpointer free_weak_ref(gpointer data)
> >  {
> > -if (weakref->object)
> > -g_object_weak_unref(weakref->object, (GWeakNotify)weak_notify_cb, 
> > weakref);
> > +GWeakRef *weakref = data;
> > +gpointer object = g_weak_ref_get(weakref);
> >  
> > +g_weak_ref_clear(weakref);
> >  g_free(weakref);
> > +if (object != NULL) {
> > +/* The main referece still exists as object is not NULL, so we can
>
> 'reference'

Fixed

>
> Looks good to me, though I'd probably return a ref to the callers, and
> add a g_object_unref() to the end of the callbacks.
>
> Acked-by: Christophe Fergeau 

Yeah, it was my first option too but I didn't like to outcome as much as
having the unref in the helper function.

Thanks, I'll be pushing it soon

toso


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH] smartcard: fix memory leak in vcard_apdu_new

[Marc-André Lureau]

thanks, applied

- Original Message -
> In the error path, 'new_apdu->a_data' is not freed.
> This can be triggered by the guest continuely.
> 
> Signed-off-by: Li Qiang 
> ---
>  src/card_7816.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/card_7816.c b/src/card_7816.c
> index b598ef9..0082504 100644
> --- a/src/card_7816.c
> +++ b/src/card_7816.c
> @@ -341,12 +341,12 @@ vcard_apdu_new(unsigned char *raw_apdu, int len,
> vcard_7816_status_t *status)
>  new_apdu->a_len = len;
>  *status = vcard_apdu_set_class(new_apdu);
>  if (*status != VCARD7816_STATUS_SUCCESS) {
> -g_free(new_apdu);
> +vcard_apdu_delete(new_apdu);
>  return NULL;
>  }
>  *status = vcard_apdu_set_length(new_apdu);
>  if (*status != VCARD7816_STATUS_SUCCESS) {
> -g_free(new_apdu);
> +vcard_apdu_delete(new_apdu);
>  new_apdu = NULL;
>  }
>  return new_apdu;
> --
> 1.8.3.1
> 
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel