[PATCH][Wok] Add support to timestamps in clone naming function
There is a potential race condiction issue when some plugin tries to use the function get_next_clone_name(all_names, basename, name_suffix=''). If clone requests are done so fast, it is possible that the plugin does not send all resource names in all_names parameter, then the function is going to select the same name for 2 or more clones, which is going to cause errors in the plugin. Supporting timestamps in microsecond precision, plugins can avoid this type of error coding its own race condiction fix or just using the timestamp function option. New name will be "basename-clone-timestamp-name-suffix". Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/wok/utils.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/wok/utils.py b/src/wok/utils.py index 7e9a928..b80a80b 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -31,6 +31,7 @@ import re import sqlite3 import subprocess import sys +import time import traceback import xml.etree.ElementTree as ET @@ -391,7 +392,7 @@ def remove_old_files(globexpr, hours): wok_log.error(str(e)) -def get_next_clone_name(all_names, basename, name_suffix=''): +def get_next_clone_name(all_names, basename, name_suffix='', ts=False): """Find the next available name for a cloned resource. If any resource named "<basename>-clone-<number><name_suffix>" is found @@ -406,26 +407,34 @@ def get_next_clone_name(all_names, basename, name_suffix=''): exist so that a resource named "foo.img" gets the name "foo-clone-1.img" instead of "foo.img-clone-1". If this parameter is used, the suffix should not be present in "basename". + ts -- use timestamp, instead of incremental numbers Return: A UTF-8 string in the format "<basename>-clone-<number><name_suffix>". """ re_group_num = 'num' - re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) - if name_suffix != '': - re_expr = u'%s%s' % (re_expr, name_suffix) + # Use timestamps or increase clone number + if ts: + # Microsecond precision + ts_suffix = int(time.time() * 1000000) + new_name = u'%s-clone-%d' % (basename, ts_suffix) + else: + re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) + if name_suffix != '': + re_expr = u'%s%s' % (re_expr, name_suffix) - max_num = 0 - re_compiled = re.compile(re_expr) + max_num = 0 + re_compiled = re.compile(re_expr) - for n in all_names: - match = re_compiled.match(n) - if match is not None: - max_num = max(max_num, int(match.group(re_group_num))) + for n in all_names: + match = re_compiled.match(n) + if match is not None: + max_num = max(max_num, int(match.group(re_group_num))) + + # increments the maximum "clone number" found + new_name = u'%s-clone-%d' % (basename, max_num + 1) - # increments the maximum "clone number" found - new_name = u'%s-clone-%d' % (basename, max_num + 1) if name_suffix != '': new_name = new_name + name_suffix -- 2.1.0
On 03/14/2016 02:54 PM, Rodrigo Trujillo wrote:
There is a potential race condiction issue when some plugin tries to use the function get_next_clone_name(all_names, basename, name_suffix=''). If clone requests are done so fast, it is possible that the plugin does not send all resource names in all_names parameter, then the function is going to select the same name for 2 or more clones, which is going to cause errors in the plugin.
Supporting timestamps in microsecond precision, plugins can avoid this type of error coding its own race condiction fix or just using the timestamp function option. New name will be "basename-clone-timestamp-name-suffix".
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/wok/utils.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/src/wok/utils.py b/src/wok/utils.py index 7e9a928..b80a80b 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -31,6 +31,7 @@ import re import sqlite3 import subprocess import sys +import time import traceback import xml.etree.ElementTree as ET
@@ -391,7 +392,7 @@ def remove_old_files(globexpr, hours): wok_log.error(str(e))
-def get_next_clone_name(all_names, basename, name_suffix=''): +def get_next_clone_name(all_names, basename, name_suffix='', ts=False):
Please, move this whole function to Kimchi. It is not related to any Wok feature so there is no reason to keep it here.
"""Find the next available name for a cloned resource.
If any resource named "<basename>-clone-<number><name_suffix>" is found @@ -406,26 +407,34 @@ def get_next_clone_name(all_names, basename, name_suffix=''): exist so that a resource named "foo.img" gets the name "foo-clone-1.img" instead of "foo.img-clone-1". If this parameter is used, the suffix should not be present in "basename". + ts -- use timestamp, instead of incremental numbers
Return: A UTF-8 string in the format "<basename>-clone-<number><name_suffix>". """ re_group_num = 'num'
- re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) - if name_suffix != '': - re_expr = u'%s%s' % (re_expr, name_suffix) + # Use timestamps or increase clone number + if ts: + # Microsecond precision + ts_suffix = int(time.time() * 1000000) + new_name = u'%s-clone-%d' % (basename, ts_suffix) + else: + re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) + if name_suffix != '': + re_expr = u'%s%s' % (re_expr, name_suffix)
- max_num = 0 - re_compiled = re.compile(re_expr) + max_num = 0 + re_compiled = re.compile(re_expr)
- for n in all_names: - match = re_compiled.match(n) - if match is not None: - max_num = max(max_num, int(match.group(re_group_num))) + for n in all_names: + match = re_compiled.match(n) + if match is not None: + max_num = max(max_num, int(match.group(re_group_num))) + + # increments the maximum "clone number" found + new_name = u'%s-clone-%d' % (basename, max_num + 1)
- # increments the maximum "clone number" found - new_name = u'%s-clone-%d' % (basename, max_num + 1) if name_suffix != '': new_name = new_name + name_suffix
Ok, please ignore this patch. Rodrigo On 03/14/2016 04:02 PM, Aline Manera wrote:
On 03/14/2016 02:54 PM, Rodrigo Trujillo wrote:
There is a potential race condiction issue when some plugin tries to use the function get_next_clone_name(all_names, basename, name_suffix=''). If clone requests are done so fast, it is possible that the plugin does not send all resource names in all_names parameter, then the function is going to select the same name for 2 or more clones, which is going to cause errors in the plugin.
Supporting timestamps in microsecond precision, plugins can avoid this type of error coding its own race condiction fix or just using the timestamp function option. New name will be "basename-clone-timestamp-name-suffix".
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/wok/utils.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/src/wok/utils.py b/src/wok/utils.py index 7e9a928..b80a80b 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -31,6 +31,7 @@ import re import sqlite3 import subprocess import sys +import time import traceback import xml.etree.ElementTree as ET
@@ -391,7 +392,7 @@ def remove_old_files(globexpr, hours): wok_log.error(str(e))
-def get_next_clone_name(all_names, basename, name_suffix=''): +def get_next_clone_name(all_names, basename, name_suffix='', ts=False):
Please, move this whole function to Kimchi. It is not related to any Wok feature so there is no reason to keep it here.
"""Find the next available name for a cloned resource.
If any resource named "<basename>-clone-<number><name_suffix>" is found @@ -406,26 +407,34 @@ def get_next_clone_name(all_names, basename, name_suffix=''): exist so that a resource named "foo.img" gets the name "foo-clone-1.img" instead of "foo.img-clone-1". If this parameter is used, the suffix should not be present in "basename". + ts -- use timestamp, instead of incremental numbers
Return: A UTF-8 string in the format "<basename>-clone-<number><name_suffix>". """ re_group_num = 'num'
- re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) - if name_suffix != '': - re_expr = u'%s%s' % (re_expr, name_suffix) + # Use timestamps or increase clone number + if ts: + # Microsecond precision + ts_suffix = int(time.time() * 1000000) + new_name = u'%s-clone-%d' % (basename, ts_suffix) + else: + re_expr = u'%s-clone-(?P<%s>\d+)' % (basename, re_group_num) + if name_suffix != '': + re_expr = u'%s%s' % (re_expr, name_suffix)
- max_num = 0 - re_compiled = re.compile(re_expr) + max_num = 0 + re_compiled = re.compile(re_expr)
- for n in all_names: - match = re_compiled.match(n) - if match is not None: - max_num = max(max_num, int(match.group(re_group_num))) + for n in all_names: + match = re_compiled.match(n) + if match is not None: + max_num = max(max_num, int(match.group(re_group_num))) + + # increments the maximum "clone number" found + new_name = u'%s-clone-%d' % (basename, max_num + 1)
- # increments the maximum "clone number" found - new_name = u'%s-clone-%d' % (basename, max_num + 1) if name_suffix != '': new_name = new_name + name_suffix
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
participants (2)
-
Aline Manera -
Rodrigo Trujillo