----- "Yolkfull Chow" <yzhou@xxxxxxxxxx> wrote:
> 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?
Yes, because random means that the UUID is generated in VM.create(),
and VM.create() is called or the destination VM, so it'll end up having
a different 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
