I'd do this a little differently, because of the constraints imposed by the different purposes of VM.create() and VM.make_qemu_command().
----- "Yolkfull Chow" <yzhou@xxxxxxxxxx> wrote:
> Signed-off-by: Yolkfull Chow <yzhou@xxxxxxxxxx>
> ---
> client/tests/kvm/kvm_vm.py | 11 +++++++++++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 503f636..895049e 100644
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -113,6 +113,13 @@ class VM:
> self.qemu_path = qemu_path
> self.image_dir = image_dir
> self.iso_dir = iso_dir
> +
> + if params.get("uuid"):
> + if params.get("uuid") == "random":
> + uuid = os.popen("cat
> /proc/sys/kernel/random/uuid").readline()
> + self.uuid = uuid.strip()
> + else:
> + self.uuid = params.get("uuid")
1. First, I'd put this code near the code that finds free ports for the
VM, immediately after the code that find a free VNC port, because this
does a similar job (assigns something dynamic to the VM).
This is not important, it's just a matter of code beauty.
2. Then, I'd change the code to something like:
if params.get("uuid") == "random":
uuid = os.popen("cat /proc/sys/kernel/random/uuid").readline()
self.random_uuid = uuid.strip()
Or consider reading the file directly:
if params.get("uuid") == "random":
file = open("/proc/sys/kernel/random/uuid")
self.random_uuid = file.read().strip()
file.close()
Not much longer, but a little nicer in my opinion.
>
> # Find available monitor filename
> @@ -374,6 +381,10 @@ class VM:
> # Make qemu command
> qemu_command = self.make_qemu_command()
>
> + # Specify the system UUID
> + if self.uuid:
> + qemu_command += " -uuid %s" % self.uuid
> +
3. Then, this code, in my opinion, should be in VM.make_qemu_command(),
not in VM.create(). You can put it at the very end, after handling
"display", or wherever you want.
It should also be changed to something like:
if params.get("uuid") == "random":
qemu_cmd += " -uuid %s" % self.random_uuid
elif params.get("uuid"):
qemu_cmd += " -uuid %s" % params.get("uuid")
4. You should add the following line to VM.__init__():
self.random_uuid = None
(I just realized that there's a little bug in the existing VM code,
so we'll have to set a few more attributes to None in VM.__init__(),
in addition to random_uuid. I'll post a patch for that.)
5. While you're at it, add a function VM.get_uuid(), which should do
something like:
def get_uuid(self):
"""
(docstring)
"""
if self.params.get("uuid") == "random":
return self.random_uuid
else:
return self.params.get("uuid")
# (Note that if there is no "uuid" in self.params,
# this function will return None, which is good)
This will be useful for a little "uuid test" Yaniv once suggested --
it will make sure the guest reports the correct uuid.
The rationale behind all this:
VM.make_qemu_command() is often called with altered 'params', to see
if those params would lead to a different qemu command.
If a VM is running with a random uuid, and we call VM.make_qemu_command()
with params that specify "uuid = ..." (not random), we want the qemu
command to reflect that. If a VM is running with a user-specified uuid,
and we want to make it random, we want the qemu command to change.
However, if a VM is running with a random uuid, and we start a new test
with a random uuid, we don't want to restart the VM with a new random uuid --
we'll just use the existing random uuid.
So, if the uuid is random, it should be generated in create(), and used
in make_qemu_command(). If it's user-specified, it should just be used by
make_qemu_command().
I may have made mistakes in the code in this message, so it'll need a
little testing.
Thanks,
Michael
> # Is this VM supposed to accept incoming migrations?
> if for_migration:
> # Find available migration port
> --
> 1.6.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
