Extract the Bash code for libvirt handling to python scripts #335
Conversation
| @@ -55,6 +55,7 @@ fi | |||
| # causes of this is violation of the common shell coding standard which | |||
| # uses uppercase for environment variables and constants, and lowercase | |||
| # for local variables. | |||
| : ${mkcloud_lib_dir:=$(dirname $0)/lib} | |||
There was a problem hiding this comment.
Maybe even more flexible:
+: ${mkcloud_dir:=$(dirname $0)}
+: mkcloud_lib_dir=${mkcloud_dir}/lib
With this we could also fetch other scripts like qa_crowbarsetup from a different location (and drop the current special variable for it).
|
@MaximilianMeister thanks for porting it to Python. Really nice to see. Maybe we can start thinking about how to test this code 👍 |
MaximilianMeister
force-pushed
the
extract-libvirt-to-python
branch
from
eee51d9 to
6962ea0
Compare
| dom.undefine() | ||
|
|
||
| machine = "{}-{}".format("qemu", vm) | ||
| machinectl = subprocess.call(["test", "-x", "/usr/bin/machinectl"]) |
There was a problem hiding this comment.
You can use p = "/usr/bin/machinectl"; os.path.isfile(p) and os.access(p , os.X_OK)
There was a problem hiding this comment.
Oh true. Technically this is enough:
machinectl = os.access('/usr/bin/machinectl'. os.X_OK)This will return False if the file doesn't exist.
Also we have sh (like in the iSCSI code). Is a bit nicer execute machinectl with sh, but subprocess is OK. Usually I skip the check if the command exists, and wrap subprocess.call intro a try to capture OSError exception in case the file do not exist.
|
I agree with all the feedback given so far, but it gets +1000 from me even before you improve it further :) /me dreams of a pure Python mkcloud ... |
|
Is the a good reason for using Python 2? From my point of view the rewrite makes more sense to do it in Python 3. So we can be sure to have no problems in the future and we can benefit from the Python 3 improvements. At the moment this is not much work but later the switch from 2 to 3 can cause many problems. |
|
Why does it have to be either 2 or 3? Can't it be written in a way which works with both? Surely that's the sensible thing to do. |
This options point to the mkcloud dir + libraries/separate scripts
This is part of the refactoring of mkcloud. All the bash code that has been used to define and start a virtual admin network is now handled in a standalone python script
This is part of the refactoring of mkcloud. All the bash code that has been used to define and start a virtual machine is now handled in a standalone python script
MaximilianMeister
force-pushed
the
extract-libvirt-to-python
branch
from
6962ea0 to
2dd3a7a
Compare
This is part of the refactoring of mkcloud. All the bash code that has been used to cleanup libvirt resources is now handled in a standalone python script
MaximilianMeister
force-pushed
the
extract-libvirt-to-python
branch
from
2dd3a7a to
c2438c4
Compare
|
👍 but wait with merge for the last test run to finish |
|
@jdsn test was passing. @aspiers, @aplanas, @toabctl, @rsalevsky are you fine with the python part? |
|
@MaximilianMeister OK from my side : ) +1 |
|
Not tested but codewise 👍 |
Extract the Bash code for libvirt handling to python scripts
This is part of the refactoring for mkcloud.
All the libvirt code that has been previously handled by Bash, should be extracted to a more suitable language for the task. We chose python over ruby here because it comes with libvirt bindings, which are available in SLES.
As I have very little experience with python, please give me some input on that.
Further I chose to write the tools as granular as possible, so that one script does only one job, which makes it more maintainable IMO. I thought about putting all libvirt stuff in one script but I didn't see an advantage in that. What do you suggest?
This PR introduces the
cleanupof libvirt resourcesnetworkdefinition and creationvmdefinition and creationThe actual creation of the XML files, as well as host specific libvirt configuring (starting the daemon etc) will follow up in this PR, or another.
But I would like to get some feedback from you before continuing.