[Kimchi-devel] [PATCH] [WoK] Asynchronous UI notification implementation
Aline Manera
alinefm at linux.vnet.ibm.com
Tue Feb 28 14:12:39 UTC 2017
On 02/28/2017 10:58 AM, Daniel Henrique Barboza wrote:
>
>
> On 02/28/2017 10:31 AM, Aline Manera wrote:
>>
>>
>> On 02/28/2017 10:24 AM, Daniel Henrique Barboza wrote:
>>>
>>>
>>> On 02/28/2017 10:02 AM, Lucio Correia wrote:
>>>> On 27/02/2017 18:30, Daniel Henrique Barboza wrote:
>>>>>
>>>>>
>>>>> On 02/27/2017 06:18 PM, Lucio Correia wrote:
>>>>>> On 27/02/2017 18:12, Daniel Henrique Barboza wrote:
>>>>>>>>>> +BASE_DIRECTORY = '/run'
>>>>>>>>> Suggestion to use:
>>>>>>>>> os.path.join('/run/user', str(os.getuid()))
>>>>>>>>> in order tests may be run with root.
>>>>>>>
>>>>>>> I would prefer to choose a path that can be written by anyone
>>>>>>> else to
>>>>>>> allow the push_server
>>>>>>> to be started without root.
>>>>>>
>>>>>> Sorry, I meant *without* root.
>>>>>> /run/user/<UID> allows for that, it's writable by the user that
>>>>>> started wokd, be it root or not.
>>>>>>
>>>>> This change would require further UI changes to allow the /config API
>>>>> (or other) to inform
>>>>> the UI of the current websocket URL. This will not solve the problems
>>>>> I've seen with the unit tests
>>>>> though - multiple instances of the push_server will not be
>>>>> possible and
>>>>> the unit tests will
>>>>> break.
>>>>
>>>> I'm not seeing the relation between that BASE_DIR and the URL used
>>>> by UI. Anyway, I believe we both agree that just one server and one
>>>> URL is fine.
>>> The unix socket, now /run/woknotifications, is used by the push
>>> server to send
>>> notifications. The websocket creates a proxy between this unix
>>> socket and a
>>> common TCP socket that the UI can read/write to.
>>>
>>> This connection is made by the following URL:
>>>
>>> <host:port>/websockify?token=<token>
>>>
>>> The UI knows what token to use to connect to the unix socket because
>>> the
>>> token is based on the file path of the unix socket. This is the
>>> relevant UI
>>> code:
>>>
>>> var token = wok.urlSafeB64Encode('woknotifications').replace(/=*$/g,
>>> "");
>>>
>>>
>>> This means that if I change the file path to /run/user_id, the token
>>> changes too. If the UI can't
>>> predict what unix socket is being used in WoK then we need to supply
>>> this info to the UI
>>> in another way.
>>
>> Daniel, for the novnc/spice/serial console, the token is the virtual
>> machine name, ie, it is not related to the real path in the system.
>> The websocket knows on which directory to look for the token and it
>> is all.
>>
>> Why is it different for the notifications? Shouldn't the directory be
>> known by the server and the UI only request a token? The same as
>> novnc/spice/serial does.
>
> In these cases the UI doesn't request a token, the UI already knows
> it. It is embedded in the
> URL.
>
> kimchi.api.js:
>
> serialToVM : function(vm) {
> (...)
> url = 'https://' + location.hostname + ':' + proxy_port;
> url += server_root;
> url += "/plugins/kimchi/serial/html/serial.html";
> url += "?port=" + proxy_port;
> url += "&path=" + server_root + "/websockify";
> url += "?token=" +
> wok.urlSafeB64Encode(vm+'-console').replace(/=*$/g, "");
> url += '&encrypt=1';
>
>
> (same thing with novnc and spice, check vncToVM and spiceToVM in the same
> file)
>
>
> In serial.html the token is retrieved as follows:
>
>
> var url = 'wss://' + window.location.hostname + ':' +
> params.get('port');
> url += '/' + params.get('path');
> url += '?token=' + params.get('token');
> var socket = new WebSocket(url, ['base64']);
>
>
>
> Note that for serial,novnc and spice the token is already knew by the
> UI due to the
> URL. In the situation we're describing with the push server this will
> not be case - we
> can't predict the current user information and embed it in the URL
> (not in the frontend
> level, at least).
>
I don't think it is true!
The UI just need to provide a token. The token is the file name. There
is no relation with the websockets directory.
Please, do a test! Change the directory on backend, proper set it on
websocket, keep the file name the same "woknotifications" and test the
async notification.
>
> This is why I am saying that we need to deliver this information in
> another fashion to
> the UI, perhaps using the /config API.
>
It is not needed.
>
>>
>>>
>>>>
>>>> My only concern is the directory to save that woknotifications file
>>>> and the permissions required for it.
>>>>
>>>>
>>>>>
>>>>> We need to discuss an alternative where:
>>>>>
>>>>> - any user can start the push_server, as you require
>>>>
>>>> I'm not requiring it. Just saying that if it will be started by
>>>> tests, it needs to run without sudo (i.e. use paths in filesystems
>>>> that allow for that).
>>>>
>>>> As I mentioned earlier, testing for 'test' option is not enough to
>>>> avoid PushServer to be started during tests, since as of now 'test'
>>>> option only governs which model will be used.
>>>
>>> Strange, it worked for me. I think you're referring to WoK changes
>>> that were made
>>> right after/before I sent this patch set. I'll re-test it and see
>>> how I can prevent it
>>> from running on test mode.
>>>
>>>>
>>>>
>>>>>
>>>>> - if we're to allow the push_server to be run in test_mode*, we
>>>>> need to
>>>>> think in a way of
>>>>> generating random paths that can be written by any user as well.
>>>>> You are
>>>>> working closely
>>>>> in Debian changes. What system dir can be used that can be written by
>>>>> any user and,
>>>>> preferably, exists in RPM distros too?
>>>>
>>>> On Fedora 25, /run/user/<UID>/libvirt is the default dir used to
>>>> store runtime data by libvirt when it is started with
>>>> "qemu:///session" url (regular user session).
>>>>
>>>> In fact it seems to be default place for that kind of usage in both
>>>> distros:
>>>>
>>>> Ubuntu 16.04:
>>>> $ ls /run/user/1000/
>>>> dbus-session gnome-shell gvfs-burn pulse systemd
>>>> upstart-dbus-bridge.4451.pid upstart-udev-bridge.2946.pid
>>>> dconf gvfs keyring speech-dispatcher upstart
>>>> upstart-file-bridge.4451.pid upstart-udev-bridge.4451.pid
>>>>
>>>> Fedora 25:
>>>> $ ls /run/user/1000/
>>>> bus libvirt systemd
>>>
>>> I see. Perhaps changing the dir to /run/user/<UID> is the right
>>> thing to do in
>>> this case. This means that we can't avoid "breaking" the current
>>> design - we'll
>>> need to use an additional API to inform the UI of the current unix
>>> socket (token)
>>> being used.
>>>
>>>
>>>>
>>>>
>>>
>>> _______________________________________________
>>> Kimchi-devel mailing list
>>> Kimchi-devel at ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>
>>
>
More information about the Kimchi-devel
mailing list