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

[Aline Manera]

On 03/11/2016 10:50 AM, Daniel Henrique Barboza wrote:
>
>
> On 03/11/2016 10:43 AM, Aline Manera wrote:
>>
>>
>> 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.
>
> I see you point. Problem is that using only OperationFailed() in the 
> code above we'll lose
> debug information because the exception message won't be logged anywhere
> (KCHVM0081E isn't receiving the error message as argument).
>
> If you want to avoid flooding the log with messages while not losing 
> debug information,
> KCHVM0081E will need to be changed to pass along the OSError message 
> to the UI too.
>

Oh! I got your point now!
I thought the OperationFailed was also returning the the OSError message.

So I agree the OperationFailed should also return the OSError message to UI.

>
> Daniel
>
>>
>>
>>
>>>>
>>>>> +
>>>>> 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
>>>
>>
>