OpenVZ Forum


Home » International » Russian » баг в vzctl ?
баг в vzctl ? [message #7401] Fri, 13 October 2006 05:23 Go to next message
Umka is currently offline  Umka
Messages: 56
Registered: September 2006
Member
Доброго времени суток.

Собственно не пойму толи это by design толи таки баг..
собственно забавная ситуация имеет место быть..
после вызова функции установки параметров на контекст - не проверяется код возврата и безусловно пытается сохранить конфиг (пусть даже в нем есть ошибки) что может приводить к созданию некоректного конфига.
Fix тривиальный - но хотелось бы услышать коментарии SWSoft..

--- vzctl-actions.c.orig 2006-10-13 05:16:05.000000000 +0300
+++ vzctl-actions.c 2006-10-13 05:16:28.000000000 +0300
@@ -833,6 +833,10 @@
ret = set_ve0(h, g_p, vps_p, cmd_p);
else
ret = set(h, veid, g_p, vps_p, cmd_p);
+
+ if (ret != 0)
+ break;
+
if (cmd_p->opt.save == YES) {
get_vps_conf_path(veid, fname, sizeof(fname));
vps_save_config(veid, fname, cmd_p, vps_p, &g_action);
Re: баг в vzctl ? [message #7409 is a reply to message #7401] Fri, 13 October 2006 07:06 Go to previous messageGo to next message
Igor Sukhih is currently offline  Igor Sukhih
Messages: 21
Registered: May 2006
Junior Member
О каких ошибках идет речь, все ошибки должны преверятся на этапе парсинга параметров (или имеются в виду конфликты)
Если ВЕ не ранится для Вас уже не важно что в конфиг попадут "некоректные" параметры?

Re: баг в vzctl ? [message #7413 is a reply to message #7409] Fri, 13 October 2006 07:29 Go to previous messageGo to next message
Umka is currently offline  Umka
Messages: 56
Registered: September 2006
Member
Igor Sukhih wrote on Fri, 13 October 2006 03:06

О каких ошибках идет речь, все ошибки должны преверятся на этапе парсинга параметров (или имеются в виду конфликты)
Если ВЕ не ранится для Вас уже не важно что в конфиг попадут "некоректные" параметры?


Собственно в оригинальном варианте у вас есть 3 точки в которых может быть вызван vzerror.
Quote:


# grep vzerror vps-functions
vzerror "Missing parameter: $VAR" $VZ_INVALID_PARAMETER_SYNTAX
vzerror "Unable to get source ip [${device}]" $VZ_CANT_ADDIP
vzerror "Unable to add route ${IP_CMD} route add $1 dev venet0 src ${src_addr}" $VZ_CANT_ADDI


кроме того это функции vzcheckvar, которые присутствуют внутри каждого из shell сприптов. Да и мало ли по какое еще причине надо выйти из shell скрипта с указазанием ошибки. Я не прав?

Кроме того любое нарушение формата логичнее проверять в той точке где оно обрабатывается. Или я не прав?

А обрабатываются настройки шейпера у меня в шел скриптах - поэтому я добавил проверок при создании объектов шейпера, На что собственно и накололся сегодня утром - когда для теста скормил строку с неправильным форматом. Результатом был вывод error`а на экран и испорченый конфиг.


Re: баг в vzctl ? [message #7414 is a reply to message #7413] Fri, 13 October 2006 07:50 Go to previous messageGo to next message
Igor Sukhih is currently offline  Igor Sukhih
Messages: 21
Registered: May 2006
Junior Member
Umka wrote on Fri, 13 October 2006 03:29

Igor Sukhih wrote on Fri, 13 October 2006 03:06

О каких ошибках идет речь, все ошибки должны преверятся на этапе парсинга параметров (или имеются в виду конфликты)
Если ВЕ не ранится для Вас уже не важно что в конфиг попадут "некоректные" параметры?


Собственно в оригинальном варианте у вас есть 3 точки в которых может быть вызван vzerror.
Quote:


# grep vzerror vps-functions
vzerror "Missing parameter: $VAR" $VZ_INVALID_PARAMETER_SYNTAX
vzerror "Unable to get source ip [${device}]" $VZ_CANT_ADDIP
vzerror "Unable to add route ${IP_CMD} route add $1 dev venet0 src ${src_addr}" $VZ_CANT_ADDI


кроме того это функции vzcheckvar, которые присутствуют внутри каждого из shell сприптов. Да и мало ли по какое еще причине надо выйти из shell скрипта с указазанием ошибки. Я не прав?



Из ошибки в шел скрипте не всегда следует что этот параметр неверный и его не следует сохранять,
как вы правильно заметили его (скрипт) могут вообше прибить, и мы получаем, чтобы сохранит параметры нужно
стопать ВЕ.
На данный момент ip адрес является единственным параметром который откатывается при ошибе.

Quote:


Кроме того любое нарушение формата логичнее проверять в той точке где оно обрабатывается. Или я не прав?


Да прав, потому они обрабатываются на входе т.е. командная строка/конфиг.

Quote:


А обрабатываются настройки шейпера у меня в шел скриптах - поэтому я добавил проверок при создании объектов шейпера, На что собственно и накололся сегодня утром - когда для теста скормил строку с неправильным форматом. Результатом был вывод error`а на экран и испорченый конфиг.



Я понял Вашу пробл., но это не тот фикс который нужен.

[Updated on: Fri, 13 October 2006 10:17] by Moderator

Report message to a moderator

Re: баг в vzctl ? [message #7415 is a reply to message #7414] Fri, 13 October 2006 08:17 Go to previous messageGo to next message
Umka is currently offline  Umka
Messages: 56
Registered: September 2006
Member
Igor Sukhih wrote on Fri, 13 October 2006 03:50

Я понял Вашу пробл., но это не тот фикс который нужен.


Вы можете более развернуто ответить в чем притензии к этому патчу?
Данный патч запрещает сохранение конфига - если команда относящаяся к VPS была выполнена с ошибкой. Это может быть ошибка формата, ошибка выполнения и тп.. Любая из этих ошибок нарушит однозначное соотвествие между текуще работающим VE и его конфигом (в который будет записано изменение).. К чему плодить рассинхронизацию?

Может Вы объясните свое виденье этой проблемы?
Re: баг в vzctl ? [message #7420 is a reply to message #7415] Fri, 13 October 2006 08:37 Go to previous messageGo to next message
Igor Sukhih is currently offline  Igor Sukhih
Messages: 21
Registered: May 2006
Junior Member
Umka wrote on Fri, 13 October 2006 04:17

Igor Sukhih wrote on Fri, 13 October 2006 03:50

Я понял Вашу пробл., но это не тот фикс который нужен.


Вы можете более развернуто ответить в чем притензии к этому патчу?
Данный патч запрещает сохранение конфига - если команда относящаяся к VPS была выполнена с ошибкой. Это может быть ошибка формата, ошибка выполнения и тп.. Любая из этих ошибок нарушит однозначное соотвествие между текуще работающим VE и его конфигом (в который будет записано изменение).. К чему плодить рассинхронизацию?


Можно получить ответ на вопрос
Если ВЕ не ранится для Вас уже не важно что в конфиг попадут "некоректные" параметры?

Все параметры проверяются на входе и они все корректны по пределению, возможны только конфликты.

[Updated on: Fri, 13 October 2006 10:11] by Moderator

Report message to a moderator

Re: баг в vzctl ? [message #7421 is a reply to message #7420] Fri, 13 October 2006 08:46 Go to previous messageGo to next message
Umka is currently offline  Umka
Messages: 56
Registered: September 2006
Member
Igor Sukhih wrote on Fri, 13 October 2006 04:37


Можно получить ответ на вопрос
Если ВЕ не ранится для Вас уже не важно что в конфиг попадут "некоректные" параметры?

Все параметры проверяются на входе и они все корректны по пределению, возможны только конфликты.


Что значит "не ранится"? выражайтесь яснее плиз.. не выполняется? - важно.

Но к сути этой ошибки это отношения не имеет. Не важно по какой причине ядро\скрипты\etc сказало что данный параметр не может быть применен - и в этом случае мы все равно будем сохранять данные в конфиг?

Re: баг в vzctl ? [message #7422 is a reply to message #7401] Fri, 13 October 2006 08:52 Go to previous messageGo to next message
kn1ght is currently offline  kn1ght
Messages: 17
Registered: May 2006
Location: Piter
Junior Member
а потом оно при рестарте VPS он не загрузится..
ИМХО логичнее не давать записывать корявые данные в конфиг, чем потом его при чтении не выполнять..

p.s. а давйте сделаем голосование за патчи? а то вот пользователям патч нужен, а разработчикам нет...

[Updated on: Fri, 13 October 2006 08:56]

Report message to a moderator

Re: баг в vzctl ? [message #7425 is a reply to message #7422] Fri, 13 October 2006 09:24 Go to previous messageGo to next message
Umka is currently offline  Umka
Messages: 56
Registered: September 2006
Member
kn1ght wrote on Fri, 13 October 2006 04:52

а потом оно при рестарте VPS он не загрузится..
ИМХО логичнее не давать записывать корявые данные в конфиг, чем потом его при чтении не выполнять..

p.s. а давйте сделаем голосование за патчи? а то вот пользователям патч нужен, а разработчикам нет...

Самое веселое дальше. Господа девелоперы - вы вобще любите обрабатывать ошибочные ситуации? или ну его нафик?
Открываем parse_devices_str() и видим в конце воооот такой кусок кода...
Quote:


ret = parse_dev_perm(mode, &dev->mask);
return 0;


я все понимаю - это тоже так надо или таки бага?
Вобще кто нить проводи аудит кода в vzctl?
Re: баг в vzctl ? [message #7427 is a reply to message #7425] Fri, 13 October 2006 09:44 Go to previous messageGo to next message
kn1ght is currently offline  kn1ght
Messages: 17
Registered: May 2006
Location: Piter
Junior Member
Quote:


я все понимаю - это тоже так надо или таки бага?
Вобще кто нить проводи аудит кода в vzctl?


а что надо? =)
Re: баг в vzctl ? [message #7431 is a reply to message #7401] Fri, 13 October 2006 10:23 Go to previous messageGo to next message
kir is currently offline  kir
Messages: 1645
Registered: August 2005
Location: Moscow, Russia
Senior Member

Попробую пояснить развёрнуто.

Текущее поведение vzctl такое:

Когда VE в состоянии running, команда vzctl set .... --save пытается приложить параметры, а потом записывает их в конфиг.

Когда VE в состоянии stopped, команда vzctl set .... --save записывает параметры в конфиг.

Таким образом, работа с конфиг-файлом не зависит от состояния VE (running/stopped).

После приложения вашего патча поведение vzctl кардинально меняется. Теперь некоторые параметры могут попасть или не попасть в конфиг в зависимости от состояния VE (running/stopped). Это нежелательно, так как вносит в систему элемент неопределённости.

В случае, если вам нужна функциональность "записать параметры в конфиг, если они прикладываются успешно", сделайте так (проверив предварительно, что VE запущена):
vzctl set $VEID $PARAMS && vzctl set $VEID $PARAMS --save


Kir Kolyshkin
http://static.openvz.org/userbars/openvz-developer.png
Re: баг в vzctl ? [message #7433 is a reply to message #7431] Fri, 13 October 2006 11:06 Go to previous messageGo to next message
Umka is currently offline  Umka
Messages: 56
Registered: September 2006
Member
kir wrote on Fri, 13 October 2006 06:23


После приложения вашего патча поведение vzctl кардинально меняется. Теперь некоторые параметры могут попасть или не попасть в конфиг в зависимости от состояния VE (running/stopped). Это нежелательно, так как вносит в систему элемент неопределённости.


Вы спутали - эта функциональность не меняется - разве что некоторые функции будут возвращать error в случае если not running. А это уж вопросы к коду который выполняет изменение параметра - этот код должен знать поведение при различных состояниях VE.
Мой патч лиш переносит нагрузку о проверке правильности внутрь функций применения параметров. Сейчас же вы отбрасываете возможные ошибки и надеетесь, что ошибки вас не касаются.
Так тоже не правильно - согласитесь?

Quote:


В случае, если вам нужна функциональность "записать параметры в конфиг, если они прикладываются успешно", сделайте так (проверив предварительно, что VE запущена):
vzctl set $VEID $PARAMS && vzctl set $VEID $PARAMS --save


Большое спасибо за урок по shell. Но в данном случае мы обсуждаем повещение vzctl при наличии ошибки, а не возможные методы обхода.
Или команда OpenVZ(SWSoft) не заинтересована в улучшении качества кода?
Re: баг в vzctl ? [message #7436 is a reply to message #7433] Fri, 13 October 2006 11:29 Go to previous messageGo to next message
kir is currently offline  kir
Messages: 1645
Registered: August 2005
Location: Moscow, Russia
Senior Member

Давайте будем разговаривать корректно. Команда заинтересована в улучшении качества кода. Я лично в этом тоже заинтересован. Мы также заинтересованы в увеличении количества сторонних разработчиков и контрибьюторов, и очень рады, когда нам шлют патчи. Не знаю, что в моём посте заставило вас думать об обратном.

Я всего лишь пытался объяснить, почему было бы некорректно не записывать параметры в конфиг в случае, если они не приложились к VE.

Быть может, я что-то не так понял. Сейчас ещё раз перечитаю весь тред от начала.


Kir Kolyshkin
http://static.openvz.org/userbars/openvz-developer.png
Re: баг в vzctl ? [message #7438 is a reply to message #7436] Fri, 13 October 2006 11:36 Go to previous messageGo to next message
kir is currently offline  kir
Messages: 1645
Registered: August 2005
Location: Moscow, Russia
Senior Member

Я ещё раз всё перечитал. Ваш патч (в первом посте этого треда) изменяет функциональность vzctl так, что он не вносит изменения в конфиг, если они не приложились. Поправьте меня, если я это не так понял.

Такое изменение нежелательно по причинам, изложенным мной выше.

Да, проблема существует, но её нельзя однозначно решить таким патчем. Необходимо, видимо, разнести функции валидации параметров и функции приложения параметров, и изменить логику на следующую:

1. проверить параметры, при ошибке -- вываливаться
2. попытаться приложить параметры
3. (как и сейчас) безусловно записать параметры в конфиг.


Kir Kolyshkin
http://static.openvz.org/userbars/openvz-developer.png
Re: баг в vzctl ? [message #7439 is a reply to message #7438] Fri, 13 October 2006 11:51 Go to previous message
Umka is currently offline  Umka
Messages: 56
Registered: September 2006
Member
kir wrote on Fri, 13 October 2006 07:36

Я ещё раз всё перечитал. Ваш патч (в первом посте этого треда) изменяет функциональность vzctl так, что он не вносит изменения в конфиг, если они не приложились. Поправьте меня, если я это не так понял.


Почти. Только сформулировать иначе. Если функция применения параметров вернула ошибку - указав основному коду, что в текущей ситуаци параметры команды не валидны.

Quote:


Такое изменение нежелательно по причинам, изложенным мной выше.

Да, проблема существует, но её нельзя однозначно решить таким патчем. Необходимо, видимо, разнести функции валидации параметров и функции приложения параметров, и изменить логику на следующую:

1. проверить параметры, при ошибке -- вываливаться
2. попытаться приложить параметры
3. (как и сейчас) безусловно записать параметры в конфиг.

4. заставить функции изменения параметров к VE разбираться - что есть фатальная ошибка (которая может повлиять на последующее поведение - в частности старт), а что не фатальная, которую можно игнорировать.

Реализовав такой алгоритм мы убираем "знание" о поведение отдельно взятой команды внутрь обработчики выполняющего эту команду. От него потребуется только одно - вернуть 0 если данные коректны и допустимы, или вернуть код ошибки которая идентифицирует проблему. Возможно это потребует правки в логике обработчиков - но IMHO это самый "дешевый" способ получить нужную функциональность.


Previous Topic: Количество процессов
Next Topic: /proc/meminfo, free и все остальное...
Goto Forum:
  


Current Time: Tue May 28 13:50:50 GMT 2024

Total time taken to generate the page: 0.01547 seconds