[Kimchi-devel] [PATCH] [Kimchi 1/4] Move unix socket files from /tmp to /run/wok

[Aline Manera]

On 03/10/2016 03:21 PM, Daniel Henrique Barboza wrote:
>
>
> On 03/10/2016 03:16 PM, Aline Manera wrote:
>>
>>
>> On 03/09/2016 04:04 PM, Jose Ricardo Ziviani wrote:
>>>   - /run is a temporary fs (tmpfs) more adequate to store this kind of
>>>     file.
>>>
>>> Signed-off-by: Jose Ricardo Ziviani <joserz at linux.vnet.ibm.com>
>>> ---
>>>   i18n.py          |  1 +
>>>   model/vms.py     | 11 ++++++++++-
>>>   serialconsole.py |  5 +++--
>>>   3 files changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/i18n.py b/i18n.py
>>> index 707b74b..e219857 100644
>>> --- a/i18n.py
>>> +++ b/i18n.py
>>> @@ -135,6 +135,7 @@ messages = {
>>>       "KCHVM0078E": _("Memory or Maximum Memory value is higher than 
>>> amount supported by the host: %(memHost)sMiB."),
>>>       "KCHVM0079E": _("Memory or Maximum Memory value is higher than 
>>> maximum amount recommended: %(value)sTiB"),
>>>       "KCHVM0080E": _("Cannot update Maximum Memory when guest is 
>>> running."),
>>
>>> +    "KCHVM0081E": _("Impossible to create /run/wok directory."),
>>
>> If you created it on run time, when uninstalling Kimchi it will 
>> remain in the system.
>>
>>>       "KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly 
>>> assigned host device %(dev_name)s."),
>>>       "KCHVMHDEV0002E": _("The host device %(dev_name)s is not 
>>> allowed to directly assign to VM."),
>>> diff --git a/model/vms.py b/model/vms.py
>>> index 195c879..cd77575 100644
>>> --- a/model/vms.py
>>> +++ b/model/vms.py
>>> @@ -1467,8 +1467,17 @@ class VMModel(object):
>>>           if not self._vm_check_serial(name):
>>>               raise OperationFailed("KCHVM0076E", {'name': name})
>>>
>>> +        if not os.path.isdir(serialconsole.BASE_DIRECTORY):
>>> +            try:
>>> +                os.mkdir(serialconsole.BASE_DIRECTORY)
>>> +
>>> +            except OSError as e:
>>
>>> + wok_log.error(e.message)
>>> +                raise OperationFailed("KCHVM0081E")
>>
>> Every exception raised is always logged into the log file so you 
>> don't need to do that.
> If I understood it right, the wok_log.error() is logging the exact 
> OSError
> message into wok_log/terminal for debugging, while the OperationFailed is
> throwing an user-friendly error message to be shown by the UI.
>
> Perhaps it's a matter of opinion. In my opinion this is a valid case 
> to have
> both.
>

My point is: when calling any WokException the error message will be 
sent out as the request response AND be logged into wok_log/terminal for 
debugging. You can check wok/src/wok/exception.py to confirm it.

So in the above case, 2 messages will be logged in the wok_log/terminal 
which does not make sense IMO.



>>
>>> +
>>> websocket.add_proxy_token(name.encode('utf-8')+'-console',
>>> -                                  '/tmp/%s' % name.encode('utf-8'), 
>>> True)
>>> + os.path.join(serialconsole.BASE_DIRECTORY,
>>> + name.encode('utf-8')), True)
>>>
>>>           try:
>>>               self._serial_procs.append(
>>> diff --git a/serialconsole.py b/serialconsole.py
>>> index 1eb48b1..47f4c2d 100644
>>> --- a/serialconsole.py
>>> +++ b/serialconsole.py
>>> @@ -35,6 +35,7 @@ from wok.plugins.kimchi import model
>>>   SOCKET_QUEUE_BACKLOG = 0
>>>   DEFAULT_TIMEOUT = 120  # seconds
>>>   CTRL_Q = '\x11'
>>> +BASE_DIRECTORY = '/run/wok'
>>>
>>>
>>>   class SocketServer(Process):
>>> @@ -61,13 +62,13 @@ class SocketServer(Process):
>>>       def __init__(self, guest_name, URI):
>>>           """Constructs a unix socket server.
>>>
>>> -        Listens to connections on /tmp/<guest name>.
>>> +        Listens to connections on /run/wok/<guest name>.
>>>           """
>>>           Process.__init__(self)
>>>
>>>           self._guest_name = guest_name
>>>           self._uri = URI
>>> -        self._server_addr = '/tmp/%s' % guest_name
>>> +        self._server_addr = os.path.join(BASE_DIRECTORY, guest_name)
>>>           if os.path.exists(self._server_addr):
>>>               wok_log.error('Cannot connect to %s due to an existing '
>>>                             'connection', guest_name)
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>