[PATCH] Specify the system UUID for VM

[PATCH] Specify the system UUID for VM

[Yolkfull Chow]

Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
---
 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")
 
 
         # 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
+
             # Is this VM supposed to accept incoming migrations?
             if for_migration:
                 # Find available migration port
-- 
1.6.2.5

Re: [PATCH] Specify the system UUID for VM

[Yolkfull Chow]

On Thu, Jul 16, 2009 at 06:26:46PM +0800, Yolkfull Chow wrote:
> 
> Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
> ---
>  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")

Sorry, forgot to initialize self.uuid.  Will resend the patch. 

>  
>  
>          # 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
> +
>              # Is this VM supposed to accept incoming migrations?
>              if for_migration:
>                  # Find available migration port
> -- 
> 1.6.2.5
> 

[PATCH] Add UUID option into kvm command line

[Yolkfull Chow]

Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
---
 client/tests/kvm/kvm_vm.py |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 503f636..48f2916 100644
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -107,6 +107,7 @@ class VM:
         @param iso_dir: The directory where ISOs reside
         """
         self.pid = None
+        self.uuid = None
 
         self.name = name
         self.params = params
@@ -287,6 +288,11 @@ class VM:
         elif params.get("display") == "nographic":
             qemu_cmd += " -nographic"
 
+        if params.get("uuid") == "random":
+            qemu_cmd += " -uuid %s" % self.uuid
+        elif params.get("uuid"):
+            qemu_cmd += " -uuid %s" % params.get("uuid")
+
         return qemu_cmd
 
 
@@ -371,6 +377,12 @@ class VM:
             if params.get("display") == "vnc":
                 self.vnc_port = kvm_utils.find_free_port(5900, 6000)
 
+            # Find random UUID if specified 'uuid = random' in config file
+            if params.get("uuid") == "random":
+                f = open("/proc/sys/kernel/random/uuid")
+                self.uuid = f.read().strip()
+                f.close()
+
             # Make qemu command
             qemu_command = self.make_qemu_command()
 
@@ -732,3 +744,15 @@ class VM:
                 self.send_key("shift-%s" % char.lower())
             else:
                 self.send_key(char)
+
+    
+    def get_uuid(self):
+        """
+        Catch UUID of the VM.
+
+        @return: None,if not specified in config file
+        """
+        if self.params.get("uuid") == "random":
+            return self.uuid
+        else:
+            return self.params.get("uuid", None)
-- 
1.6.2.5

Re: [PATCH] Add UUID option into kvm command line

[Lucas Meneghel Rodrigues]

On Fri, 2009-07-17 at 23:17 +0800, Yolkfull Chow wrote:
> Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
> ---
>  client/tests/kvm/kvm_vm.py |   24 ++++++++++++++++++++++++
>  1 files changed, 24 insertions(+), 0 deletions(-)

Ok, I've followed the discussion around this patch, thanks Yolkfull and
Michael. Applied.

> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 503f636..48f2916 100644
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -107,6 +107,7 @@ class VM:
>          @param iso_dir: The directory where ISOs reside
>          """
>          self.pid = None
> +        self.uuid = None
>  
>          self.name = name
>          self.params = params
> @@ -287,6 +288,11 @@ class VM:
>          elif params.get("display") == "nographic":
>              qemu_cmd += " -nographic"
>  
> +        if params.get("uuid") == "random":
> +            qemu_cmd += " -uuid %s" % self.uuid
> +        elif params.get("uuid"):
> +            qemu_cmd += " -uuid %s" % params.get("uuid")
> +
>          return qemu_cmd
>  
> 
> @@ -371,6 +377,12 @@ class VM:
>              if params.get("display") == "vnc":
>                  self.vnc_port = kvm_utils.find_free_port(5900, 6000)
>  
> +            # Find random UUID if specified 'uuid = random' in config file
> +            if params.get("uuid") == "random":
> +                f = open("/proc/sys/kernel/random/uuid")
> +                self.uuid = f.read().strip()
> +                f.close()
> +
>              # Make qemu command
>              qemu_command = self.make_qemu_command()
>  
> @@ -732,3 +744,15 @@ class VM:
>                  self.send_key("shift-%s" % char.lower())
>              else:
>                  self.send_key(char)
> +
> +    
> +    def get_uuid(self):
> +        """
> +        Catch UUID of the VM.
> +
> +        @return: None,if not specified in config file
> +        """
> +        if self.params.get("uuid") == "random":
> +            return self.uuid
> +        else:
> +            return self.params.get("uuid", None)

Re: [PATCH] Specify the system UUID for VM

[Avi Kivity]

On 07/16/2009 01:26 PM, Yolkfull Chow wrote:
> Signed-off-by: Yolkfull Chow<yzhou@redhat.com>
> ---
>   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()
>    

instead of os.popen("cat ..."), you can open the file directly:

    uuid = file('/proc/.../uuid').readline()

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] Specify the system UUID for VM

[Lucas Meneghel Rodrigues]

On Wed, Jul 29, 2009 at 9:18 AM, Avi Kivity<avi@redhat.com> wrote:
> On 07/16/2009 01:26 PM, Yolkfull Chow wrote:
>>
>> Signed-off-by: Yolkfull Chow<yzhou@redhat.com>
>> ---
>>  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()
>>
>
> instead of os.popen("cat ..."), you can open the file directly:
>
>   uuid = file('/proc/.../uuid').readline()

Oooops... I didn't notice this little issue. Will make a quick patch
to change it, thanks Avi!

Re: [PATCH] Specify the system UUID for VM

[Lucas Meneghel Rodrigues]

On Wed, Jul 29, 2009 at 9:36 AM, Lucas Meneghel Rodrigues<lmr@redhat.com> wrote:
> On Wed, Jul 29, 2009 at 9:18 AM, Avi Kivity<avi@redhat.com> wrote:
>> On 07/16/2009 01:26 PM, Yolkfull Chow wrote:
>>>
>>> Signed-off-by: Yolkfull Chow<yzhou@redhat.com>
>>> ---
>>>  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()
>>>
>>
>> instead of os.popen("cat ..."), you can open the file directly:
>>
>>   uuid = file('/proc/.../uuid').readline()
>
> Oooops... I didn't notice this little issue. Will make a quick patch
> to change it, thanks Avi!

Hmm, turns out we've had sorted this out during the review process of
this particular patch. The version that got in reads the file directly
:)

Re: [Autotest] [PATCH] Specify the system UUID for VM

[Michael Goldish]

----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote:

> On Wed, Jul 29, 2009 at 9:18 AM, Avi Kivity<avi@redhat.com> wrote:
> > On 07/16/2009 01:26 PM, Yolkfull Chow wrote:
> >>
> >> Signed-off-by: Yolkfull Chow<yzhou@redhat.com>
> >> ---
> >>  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()
> >>
> >
> > instead of os.popen("cat ..."), you can open the file directly:
> >
> >   uuid = file('/proc/.../uuid').readline()
> 
> Oooops... I didn't notice this little issue. Will make a quick patch
> to change it, thanks Avi!

I think I already commented on this and Yolkfull posted a new patch,
and that patch is the one that got applied.  That's what I see in my
local tree anyway.

> _______________________________________________
> Autotest mailing list
> Autotest@test.kernel.org
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Re: [Autotest] [PATCH] Specify the system UUID for VM

[Lucas Meneghel Rodrigues]

On Wed, Jul 29, 2009 at 9:46 AM, Michael Goldish<mgoldish@redhat.com> wrote:
>
> ----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote:
>
>> On Wed, Jul 29, 2009 at 9:18 AM, Avi Kivity<avi@redhat.com> wrote:
>> > On 07/16/2009 01:26 PM, Yolkfull Chow wrote:
>> >>
>> >> Signed-off-by: Yolkfull Chow<yzhou@redhat.com>
>> >> ---
>> >>  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()
>> >>
>> >
>> > instead of os.popen("cat ..."), you can open the file directly:
>> >
>> >   uuid = file('/proc/.../uuid').readline()
>>
>> Oooops... I didn't notice this little issue. Will make a quick patch
>> to change it, thanks Avi!
>
> I think I already commented on this and Yolkfull posted a new patch,
> and that patch is the one that got applied.  That's what I see in my
> local tree anyway.

Yep, I over-reacted :)

Re: [Autotest] [PATCH] Specify the system UUID for VM

[Avi Kivity]

On 07/29/2009 03:46 PM, Michael Goldish wrote:
> I think I already commented on this and Yolkfull posted a new patch,
> and that patch is the one that got applied.  That's what I see in my
> local tree anyway.
>    

Yeah, I have a long backlog (and don't read all autotest patches 
anyway).  Sorry about the noise.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] Specify the system UUID for VM

[Yolkfull Chow]

On Wed, Jul 29, 2009 at 03:18:51PM +0300, Avi Kivity wrote:
> On 07/16/2009 01:26 PM, Yolkfull Chow wrote:
>> Signed-off-by: Yolkfull Chow<yzhou@redhat.com>
>> ---
>>   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()
>>    
>
> instead of os.popen("cat ..."), you can open the file directly:
>
>    uuid = file('/proc/.../uuid').readline()

Yes, Lucas also suggested this method as well. Since the patch has been applied, need I submit a
little patch for this? 
Thanks for suggestion. :-)

>
> -- 
> error compiling committee.c: too many arguments to function
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Specify the system UUID for VM

[Yolkfull Chow]

On Wed, Jul 29, 2009 at 09:06:25PM +0800, Yolkfull Chow wrote:
> On Wed, Jul 29, 2009 at 03:18:51PM +0300, Avi Kivity wrote:
> > On 07/16/2009 01:26 PM, Yolkfull Chow wrote:
> >> Signed-off-by: Yolkfull Chow<yzhou@redhat.com>
> >> ---
> >>   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()
> >>    
> >
> > instead of os.popen("cat ..."), you can open the file directly:
> >
> >    uuid = file('/proc/.../uuid').readline()
> 
> Yes, Lucas also suggested this method as well. Since the patch has been applied, need I submit a
> little patch for this? 
> Thanks for suggestion. :-)

Seems the answer emerges. ;-)

> 
> >
> > -- 
> > error compiling committee.c: too many arguments to function
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Specify the system UUID for VM

[Yolkfull Chow]

Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
---
 client/tests/kvm/kvm_vm.py |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 503f636..5f81965 100644
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -113,6 +113,14 @@ class VM:
         self.qemu_path = qemu_path
         self.image_dir = image_dir
         self.iso_dir = iso_dir
+        
+        self.uuid = None
+        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")
 
 
         # Find available monitor filename
@@ -374,6 +382,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
+
             # Is this VM supposed to accept incoming migrations?
             if for_migration:
                 # Find available migration port
-- 
1.6.2.5

Re: [PATCH] Specify the system UUID for VM

[Michael Goldish]

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@redhat.com> wrote:

> Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
> ---
>  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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Specify the system UUID for VM

[Yolkfull Chow]

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@redhat.com> wrote:
> 
> > Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
> > ---
> >  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@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Specify the system UUID for VM

[Michael Goldish]

----- "Yolkfull Chow" <yzhou@redhat.com> 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@redhat.com> wrote:
> > 
> > > Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
> > > ---
> > >  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@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html