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

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(a)linux.vnet.ibm.com&gt; >>>> --- >>>> 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.

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(a)ovirt.org >>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >>> >> >> _______________________________________________ >> Kimchi-devel mailing list >> Kimchi-devel(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >> >