On Thu, Jul 16, 2009 at 08:49:30AM -0400, Michael Goldish wrote:
> 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")
So when running migration test, we have to specify a static UUID for
VM,right?
>
> 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
