search

xapi-project / xcp-networkd

The XCP networking daemon
Other
14 stars 42 forks source link

CA-236855: VM.clean_shutdown task not completing on slave #140

Closed YarsinCitrix closed 6 years ago

YarsinCitrix commented 6 years ago

According to the discussion (also recorded in the ticket), this change will use the newly created read_timeout and write_timeout to process input and output from outside. Some updates according to Nanjing peer review.

Modified test case to remove the cases that is not applied.

Signed-off-by: YarsinCitrix yarsin.he@citrix.com

minishrink commented 6 years ago

One suggestion: running this through ocp-indent, as you've got some large indentations in there.

mseri commented 6 years ago

@minishrink we cannot, networkd still uses tabs indentation.

YarsinCitrix commented 6 years ago

I have updated the change according to the discussion/review comments. I addressed most of them, however some not address listed below:

  1. The ns number. I do not think there is many opportunity this number get changed, and even we do, it is once/twice on the server. But the code will be executed for many times, so I do not want to trade of the code simplicity for rare chance of configuration, say make it ms there. Also, I am not sure how to make it right int64, as the args does not support int64 and I am not sure introducing int64 to the configuration part might be risky.
  2. The End_of_file exception will not be thrown in current implementation, as read ends by returning 0 and the logic ends there.
krizex commented 6 years ago

Looks good to me! Please update the title of commit https://github.com/xapi-project/xcp-networkd/pull/140/commits/27517bbe7bc49b59d7496a4cefc22fc4943323d8 Cut off (#1) 😂

robhoes commented 6 years ago

I've done a squash+merge.